On Sat, 18 Jun 2016, at 07:07 PM, Ahmed S. Darwish wrote: > Hi :-) > > Compiling pulse master with gcc v6.1.1 emits several warnings from > the `padsp' utility. I was going to send a fixing patch, but upon > further inspection most of them seems to be problems either from > GCC itself, or from its zealous interpreatation of glibc headers. > > Do we want to release a v9.0 with such warnings? Your input is > appreciated... > > > ==> GCC -Wlogical-op not probably understanding macros > ------------------------------------------------------ > > The first warning is the following: > > utils/padsp.c: In function â??sndstat_openâ??: > utils/padsp.c:1440:9: warning: logical â??andâ?? of equal expressions > [-Wlogical-op] > && flags != (O_RDONLY|O_LARGEFILE) > ^~ > > and it comes from below if condition: > > if (flags != O_RDONLY > #ifdef O_LARGEFILE > && flags != (O_RDONLY|O_LARGEFILE) > #endif > ) > > Since O_RDONLY and O_LARGEFILE effectively have the value of > _zero_ in modern Linux systems, thus it complains because we are > effectively doing: > > if (flags != 0 && flags != 0) > > Nonetheless, gcc should've been smart enough to detect we're > comparing flags with different macros. Other projects also raised > a similar concern, and this is tracked in the following GCC > bugzilla entry: > > "Over-ambitious logical-op warning on EAGAIN vs EWOULDBLOCK" > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69602 > > > ==> GCC's tough interpreatation of __nonnull arguments > ------------------------------------------------------ > > The second class of warnigns is the following: > > utils/padsp.c: In function â??accessâ??: > utils/padsp.c:2450:8: warning: nonnull argument â??pathnameâ?? compared to > NULL [-Wnonnull-compare] > if (!pathname || > ^ > ... and similar other 7 warnings ... > > Here padsp is re-implementing libc's access(3) function. GCC is > complaining because glibc's declaration of access() marks the > first string parameter with gcc's __attribute__((nonnull)): > > /* /usr/include/unistd.h */ > extern int access (const char *__name, int __type) __THROW __nonnull > ((1)); > > The same warnings are also issued for padsp's re-implementation > of stat(), stat64(), __xstat(), etc. > > This is even more strange because the GCC nonnull attribute docs > state that "The compiler may also choose to make optimizations > based on the knowledge that certain function arguments will not be > null": > > https://gcc.gnu.org/onlinedocs/gcc-6.1.0/gcc/Common-Function-Attributes.html > > And this is stated to be the main reason of the new GCC 6 warning. > Namely, that GCC __will optimize away__ the NULL checks and the > main reason of this warning is to warn the developer about that: > > "GCC might optimize such checks away so warn the user when the > function contains such comparisions." > https://gcc.gnu.org/ml/gcc-patches/2015-09/msg00621.html > > "So the new warning essentially says 'you promised this check > is redundant and it will be removed'." > https://gnu.wildebeest.org/blog/mjw/2016/02/15/looking-forward-to-gcc6-many-new-warnings/#comment-5716 > > So what is the suggested action for pulse here? For now, we're at > odds with the system call headers provided by glibc. I don't think I want to try to fix these for 9.0 (which I hope to roll out today). GCC 6 does seem to have become a bit overzealous, so I'd rather see if that changes by 10.0. I'm interested to hear packagers' opinions about this too, though. -- Arun
