Skip to content

huawei-ups2000: fightwarn hypercorrectionism cleanup#1465

Merged
jimklimov merged 5 commits intonetworkupstools:masterfrom
biergaizi:ups2000-cleanup
Jul 13, 2022
Merged

huawei-ups2000: fightwarn hypercorrectionism cleanup#1465
jimklimov merged 5 commits intonetworkupstools:masterfrom
biergaizi:ups2000-cleanup

Conversation

@biergaizi
Copy link
Copy Markdown
Contributor

@biergaizi biergaizi commented Jul 11, 2022

In Fightwarm fix #1216, several bugs have been fixed, but it also introduced some changes that I perceive as unnecessary hypercorrectionism. For example, it used three type casts when it's only necessary to use just one type cast (as far as I can see, it's the case, but the CI/CD should let us know if the compiler disagrees). This is a minor cleanup Pull Request to remove these changes.

@biergaizi biergaizi changed the title huawei-ups2000: fightwarn hypercorrectism cleanup huawei-ups2000: fightwarn hypercorrectionism cleanup Jul 11, 2022
@biergaizi
Copy link
Copy Markdown
Contributor Author

biergaizi commented Jul 11, 2022

CI/CD has failed, but it's actually caused by an unrelated spellcheck regression in the current branch, and it has nothing to do with the actual code. Please merge the fix in #1466 then we can restart the CI/CD again.

…bit-shifting

The initial code didn't cast to uint16_t and was certainly a bug, but
to fix that, only a single cast is needed, not three casts.

Signed-off-by: Yifeng Li <tomli@tomli.me>
… casts in crc16 calculations

The initial code didn't cast to uint16_t and was certainly a bug, but
to fix that, only a single cast is needed, not three casts.

Signed-off-by: Yifeng Li <tomli@tomli.me>
This removes the unnecessary casts in between. Also, size_t is
semantically more correct than uint16_t.

Signed-off-by: Yifeng Li <tomli@tomli.me>
After crc16() has been modified to accept a size_t length,
it's unnecessary to check whether the input length is greater
than UINT16_MAX.

Also, the check on whether ident_response_len is at least
as long as the IDENT_RESPONSE_CRC_LEN is useless, since this
condition is already implied by the previous check in step 2,
it's thus removed.

Signed-off-by: Yifeng Li <tomli@tomli.me>
…e-check.

Signed-off-by: Yifeng Li <tomli@tomli.me>
@jimklimov
Copy link
Copy Markdown
Member

CI faults in last iteration were due to CI build farm slowness it seems (some daemons did not start/connect within timeout); a previous iteration (merge from master, not rebased as done later) succeeded fully. Merging, thanks :)

@jimklimov jimklimov merged commit b958188 into networkupstools:master Jul 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants