Skip to content

Fix more C warnings #1159

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 16, 2017
Merged

Fix more C warnings #1159

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CoreFoundation/Base.subproj/CFUtilities.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1519,7 +1519,7 @@ CFDictionaryRef __CFGetEnvironment() {
static CFMutableDictionaryRef envDict = NULL;
dispatch_once(&once, ^{
#if DEPLOYMENT_TARGET_MACOSX || DEPLOYMENT_TARGET_EMBEDDED
extern char ***_NSGetEnviron();
extern char ***_NSGetEnviron(void);
char **envp = *_NSGetEnviron();
#elif DEPLOYMENT_TARGET_FREEBSD || TARGET_OS_CYGWIN
extern char **environ;
Expand Down
4 changes: 2 additions & 2 deletions CoreFoundation/Locale.subproj/CFDateFormatter.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ static Boolean useTemplatePatternGenerator(CFLocaleRef locale, void(^work)(UDate
if (CFStringGetCString(ln, buffer, BUFFER_SIZE, kCFStringEncodingASCII)) localeName = buffer;
}

static void (^flushCache)() = ^{
static void (^flushCache)(void) = ^{
__cficu_udatpg_close(ptg);
ptg = NULL;
free((void *)ptgLocaleName);
Expand Down Expand Up @@ -1506,7 +1506,7 @@ static UDate __CFDateFormatterCorrectTimeToARangeAroundCurrentDate(UCalendar *ca
}
} else {
if (period < INT_MAX && futureMax > period) {
futureRange.location = 1,
futureRange.location = 1;
futureRange.length = futureMax - period;
}
if (pastMin <= 0) {
Expand Down
27 changes: 9 additions & 18 deletions CoreFoundation/RunLoop.subproj/CFRunLoop.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,8 +250,7 @@ CF_PRIVATE void __CFRestartAllThreads(CFArrayRef threads) {

static uint32_t __CF_last_warned_port_count = 0;

static void foo() __attribute__((unused));
static void foo() {
static void __attribute__((unused)) foo() {
uint32_t pcnt = __CFGetProcessPortCount();
if (__CF_last_warned_port_count + 1000 < pcnt) {
CFArrayRef threads = __CFStopAllThreads();
Expand Down Expand Up @@ -1755,32 +1754,28 @@ void CFRunLoopAddCommonMode(CFRunLoopRef rl, CFStringRef modeName) {

#if __HAS_DISPATCH__

static void __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__() __attribute__((noinline));
static void __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__(void *msg) {
static void __attribute__((noinline)) __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__(void *msg) {
_dispatch_main_queue_callback_4CF(msg);
asm __volatile__(""); // thwart tail-call optimization
}

#endif

static void __CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__() __attribute__((noinline));
static void __CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__(CFRunLoopObserverCallBack func, CFRunLoopObserverRef observer, CFRunLoopActivity activity, void *info) {
static void __attribute__((noinline)) __CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__(CFRunLoopObserverCallBack func, CFRunLoopObserverRef observer, CFRunLoopActivity activity, void *info) {
if (func) {
func(observer, activity, info);
}
asm __volatile__(""); // thwart tail-call optimization
}

static void __CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__() __attribute__((noinline));
static void __CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__(CFRunLoopTimerCallBack func, CFRunLoopTimerRef timer, void *info) {
static void __attribute__((noinline)) __CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__(CFRunLoopTimerCallBack func, CFRunLoopTimerRef timer, void *info) {
if (func) {
func(timer, info);
}
asm __volatile__(""); // thwart tail-call optimization
}

static void __CFRUNLOOP_IS_CALLING_OUT_TO_A_BLOCK__() __attribute__((noinline));
static void __CFRUNLOOP_IS_CALLING_OUT_TO_A_BLOCK__(void (^block)(void)) {
static void __attribute__((noinline)) __CFRUNLOOP_IS_CALLING_OUT_TO_A_BLOCK__(void (^block)(void)) {
if (block) {
block();
}
Expand Down Expand Up @@ -1836,8 +1831,7 @@ static Boolean __CFRunLoopDoBlocks(CFRunLoopRef rl, CFRunLoopModeRef rlm) { // C
}

/* rl is locked, rlm is locked on entrance and exit */
static void __CFRunLoopDoObservers() __attribute__((noinline));
static void __CFRunLoopDoObservers(CFRunLoopRef rl, CFRunLoopModeRef rlm, CFRunLoopActivity activity) { /* DOES CALLOUT */
static void __attribute__((noinline)) __CFRunLoopDoObservers(CFRunLoopRef rl, CFRunLoopModeRef rlm, CFRunLoopActivity activity) { /* DOES CALLOUT */
CHECK_FOR_FORK();

CFIndex cnt = rlm->_observers ? CFArrayGetCount(rlm->_observers) : 0;
Expand Down Expand Up @@ -1904,16 +1898,14 @@ static void __CFRunLoopCollectSources0(const void *value, void *context) {
}
}

static void __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__() __attribute__((noinline));
static void __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__(void (*perform)(void *), void *info) {
static void __attribute__((noinline)) __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__(void (*perform)(void *), void *info) {
if (perform) {
perform(info);
}
asm __volatile__(""); // thwart tail-call optimization
}

static void __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE1_PERFORM_FUNCTION__() __attribute__((noinline));
static void __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE1_PERFORM_FUNCTION__(
static void __attribute__((noinline)) __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE1_PERFORM_FUNCTION__(
#if DEPLOYMENT_TARGET_MACOSX || DEPLOYMENT_TARGET_EMBEDDED || DEPLOYMENT_TARGET_EMBEDDED_MINI
void *(*perform)(void *msg, CFIndex size, CFAllocatorRef allocator, void *info),
mach_msg_header_t *msg, CFIndex size, mach_msg_header_t **reply,
Expand Down Expand Up @@ -1997,8 +1989,7 @@ CF_INLINE void __CFRunLoopDebugInfoForRunLoopSource(CFRunLoopSourceRef rls) {
}

// msg, size and reply are unused on Windows
static Boolean __CFRunLoopDoSource1() __attribute__((noinline));
static Boolean __CFRunLoopDoSource1(CFRunLoopRef rl, CFRunLoopModeRef rlm, CFRunLoopSourceRef rls
static Boolean __attribute__((noinline)) __CFRunLoopDoSource1(CFRunLoopRef rl, CFRunLoopModeRef rlm, CFRunLoopSourceRef rls
#if DEPLOYMENT_TARGET_MACOSX || DEPLOYMENT_TARGET_EMBEDDED || DEPLOYMENT_TARGET_EMBEDDED_MINI
, mach_msg_header_t *msg, CFIndex size, mach_msg_header_t **reply
#endif
Expand Down
2 changes: 1 addition & 1 deletion CoreFoundation/String.subproj/CFString.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oof! how was the previous even valid C?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@spevans / @phausler I think the original code is correct. The comma operator in C evaluates the first expression, discards the result and then evaluates the second and the overall expression evaluates to that (the value of the second expr).

Copy link
Contributor

@weissi weissi Aug 23, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, sorry, think I'm wrong. If CFUniCharIsMemberOfBitmap(str1Char, graphemeBMP) doesn't have any side-effects, the original code looks wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI All I did in Xcode9 was 'Update to recommended settings' which enabled the latest clang warnings so I was just fixing those up.


if (str1Char < 0x0510) {
while (++str1Index < maxStr1Index) if (!CFUniCharIsMemberOfBitmap(CFStringGetCharacterFromInlineBuffer(&inlineBuf1, str1Index), graphemeBMP)) break;
Expand Down