Fix more C warnings #1159
Fix more C warnings #1159
Conversation
- Merge function prototypes with the declaration that follows on the next line to simplify declaration when the arguments differ due to preprocessor arguments. - Fix use of comma in while statement.
|
@swift-ci please test |
1 similar comment
|
@swift-ci please test |
|
We're in the process of a giant merge of CF stuff right now, so let's hold off on this for a bit. |
| @@ -3233,7 +3233,7 @@ Boolean CFStringFindWithOptionsAndLocale(CFStringRef string, CFStringRef stringT | |||
|
|
|||
| do { | |||
| str1Char = CFStringGetCharacterFromInlineBuffer(&inlineBuf1, --index); | |||
| } while (CFUniCharIsMemberOfBitmap(str1Char, graphemeBMP), (rangeToSearch.location < index)); | |||
| } while (CFUniCharIsMemberOfBitmap(str1Char, graphemeBMP) && (rangeToSearch.location < index)); | |||
There was a problem hiding this comment.
oof! how was the previous even valid C?
There was a problem hiding this comment.
I had to look it up! The first call is always executed then the result of the function after the , is evaluated for the while loop. I assume && is the correct operator here.
There was a problem hiding this comment.
doing some testing here; this is actually a bug btw... the first function is called but the result is discarded, if you mark the CFUniCharIsMemberOfBitmap as __attribute__((pure)) or better yet __attribute__((const)) (which it is most appropriately delineated as) it ends up emitting a warning that the result is not used.
As Tony mentioned we are in the process of merging a bunch of CF things - I will keep an eye on this particular portion; file a bug on the owner and get back to you on this. We should probably fix it somehow but I need to validate exactly the right fix.
There was a problem hiding this comment.
oh, sorry, think I'm wrong. If CFUniCharIsMemberOfBitmap(str1Char, graphemeBMP) doesn't have any side-effects, the original code looks wrong.
There was a problem hiding this comment.
FYI All I did in Xcode9 was 'Update to recommended settings' which enabled the latest clang warnings so I was just fixing those up.
|
@alblue I don't think the merge has happened yet, I thought it was going to be the public bits of 10.13 but I don't see anything in the commit log. |
|
@swift-ci please test and merge |
Merge function prototypes with the declaration that follows
on the next line to simplify declaration when the arguments
differ due to preprocessor arguments.
Fix use of comma in while statement.
This is the second PR of C fixes along with #1157 after which the Xcode project can be updated to recommended settings to include the latest Clang warnings.
The
whilestatement definitely needs checking, I went with&&instead of||but a 2nd opinion would be good.