Fixed most all of the reasonable clang-tidy warnings #1479
Conversation
| else if (r != 4 || str.desc.bLength < 4) | ||
| return LIBUSB_ERROR_IO; | ||
| else if (str.desc.bDescriptorType != LIBUSB_DT_STRING) | ||
| else if (r != 4 || str.desc.bLength < 4 || str.desc.bDescriptorType != LIBUSB_DT_STRING) |
There was a problem hiding this comment.
what was the warning here?
I personally find the "before" more descriptive/readable
There was a problem hiding this comment.
| else if (r < DESC_HEADER_LENGTH || str.desc.bLength > r) | ||
| return LIBUSB_ERROR_IO; | ||
| else if (str.desc.bDescriptorType != LIBUSB_DT_STRING) | ||
| else if (r < DESC_HEADER_LENGTH || str.desc.bLength > r || str.desc.bDescriptorType != LIBUSB_DT_STRING) |
|
I appreciate the systematic approach, and I understand the motivation to silence clang-tidy, but I would prefer the commit summary to say what change is done*, or how it makes the code better, independent of clang-tidy. Silencing clang-tidy is of course a nice side-effect :) And it is important to note if some commits only affects examples or some platform, it makes it much easier for someone to peruse the git log and skip things that are not of their interest. If small trivial fixes to an example file can be grouped together that is fine too. *) Generally it should express the intention do something, not what one thinks has been done. It is not always the same :) E.g.: -> examples/ezusb: Replace rewind, which gives no error result, with fseek, which does |
Some of them do, but yeah ok I'll improve all the commit messages with more detail. |
|
All the summaries (first line) say "Fixed all clang-tidy...". |
Oh missed that, it's the summaries you don't like... I guess I'm so familiar with their names that it seems clear to me. I'll reword them... |
No, you don't need to reword the names of the clang-tidy keywords if that is what you mean. But they should be relegated to the message, not the summary. Please see my example above. |
|
OK done, hopefully you like it better now. |
|
I added those prefix things you like |
…ich does. Fixes all clang-tidy bugprone-unsafe-functions warnings
The C langugage reserves various identifiers for itself that user code must not use. Fixes all clang-tidy bugprone-reserved-identifier warnings Also, 4 of 5 file extension tests were case insensitive, and 1 was not. Changed it to be insensitive too.
…any lower-precedence operators from the surrounding expression Fixes all clang-tidy bugprone-macro-parentheses warnings
Took the name from the .c file. Fixes all clang-tidy readability-inconsistent-declaration-parameter-name warnings
All such warnings are already fixed in master.
Depending on the font, an l suffix can look like a 1. Consider "231l". Thus prefer uppercase. Fixes all clang-tidy readability-uppercase-literal-suffix warnings
Fixes all clang-tidy bugprone-assignment-in-if-condition warnings
Copy-pasting can often result in mistakes like:
```
if (test_value(x)) {
y++;
do_something(x, y);
} else {
y++;
do_something(x, y);
}
```
Thus it is preferable that branch bodies be unique.
Fixes all clang-tidy bugprone-branch-clone warnings
…or 1 Fixes all clang-tidy bugprone-suspicious-string-compare warnings
Fixes all clang-tidy readability-non-const-parameter warnings
These multiplication could indeed have overflowed, but now they are performed with a bigger type, matching the type they are ultimately stored in. Fixes all clang-tidy bugprone-implicit-widening-of-multiplication-result warnings
…iterated Fixes all clang-tidy bugprone-too-small-loop-variable warnings
The addition could overflow, the upcast needs to be performed before, not after. Fixes all clang-tidy bugprone-misplaced-widening-cast warnings
|
@tormodvolden hopefully we can get this one merged next? |
No description provided.