From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/13438 Path: news.gmane.org!.POSTED!not-for-mail From: Sedrubal Newsgroups: gmane.linux.lib.musl.general Subject: Re: [PATCH] Fix many compiler warnings Date: Tue, 13 Nov 2018 01:15:11 +0100 Message-ID: <1676444.u5bekTFY47@prometheus.sedrubal.de> References: <1636016.5PCnkOXM1K@prometheus.sedrubal.de> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/signed; boundary="nextPart5961470.eLiFrnERWP"; micalg="pgp-sha256"; protocol="application/pgp-signature" X-Trace: blaine.gmane.org 1542068011 20062 195.159.176.226 (13 Nov 2018 00:13:31 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Tue, 13 Nov 2018 00:13:31 +0000 (UTC) To: musl@lists.openwall.com Original-X-From: musl-return-13454-gllmg-musl=m.gmane.org@lists.openwall.com Tue Nov 13 01:13:27 2018 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by blaine.gmane.org with smtp (Exim 4.84_2) (envelope-from ) id 1gMMKY-00051x-7c for gllmg-musl@m.gmane.org; Tue, 13 Nov 2018 01:13:22 +0100 Original-Received: (qmail 9891 invoked by uid 550); 13 Nov 2018 00:15:30 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 9852 invoked from network); 13 Nov 2018 00:15:29 -0000 X-Virus-Scanned: amavisd-new at heinlein-support.de In-Reply-To: <1636016.5PCnkOXM1K@prometheus.sedrubal.de> Xref: news.gmane.org gmane.linux.lib.musl.general:13438 Archived-At: --nextPart5961470.eLiFrnERWP Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Thanks for your feedback. > Well, hope belongs in the church. Whether or not you introduced bugs is > a thing you can test for. There is libc-test. You're right, I'm sorry. Unfortunately for me libc-test contains 5 errors for master, v1.1.20 and for master+my changes. So I'm not sure, if I broke something... > In any case, changes like this should be motivated by "this change makes > something easier to understand", not by "the compiler printed a > warning". Most likely you're right. But I thought, it's quite easy to fix these warnings and maybe fixing them helps people to understand the code. I've to confess that I'm not really familiar with C but right now I need tables like this [1] to understand your code... Maybe I'm not the only one... > Indeed the above is wrong, but this might be one of the few changes we > could actually consider doing (with it fixed to be right) since it's > in a public header and might impact building programs against musl. Yes, that's the reason, why I created this patch... I'm OK if you only apply some of the bits of the patch that belong to headers and that you like. Below you can find an updated version of my patch. Cheers, Sedrubal [1] https://en.wikipedia.org/wiki/ Operators_in_C_and_C%2B%2B#Operator_precedence P.S.: Sorry for breaking the thread. I didn't subscribe to the mailing list before and I didn't manage to find out the message ID of the last mail in the thread, so I replied on my own mail... --- diff --git a/include/byteswap.h b/include/byteswap.h index 00b9df3c..5dad8351 100644 --- a/include/byteswap.h +++ b/include/byteswap.h @@ -6,17 +6,17 @@ static __inline uint16_t __bswap_16(uint16_t __x) { - return __x<<8 | __x>>8; + return (__x << 8) | (__x >> 8); } static __inline uint32_t __bswap_32(uint32_t __x) { - return __x>>24 | __x>>8&0xff00 | __x<<8&0xff0000 | __x<<24; + return (__x >> 24) | (__x>>8 & 0xff00) | (__x<<8 & 0xff0000) | (__x << 24); } static __inline uint64_t __bswap_64(uint64_t __x) { - return __bswap_32(__x)+0ULL<<32 | __bswap_32(__x>>32); + return ((__bswap_32(__x) + 0ULL) << 32) | __bswap_32(__x >> 32); } #define bswap_16(x) __bswap_16(x) diff --git a/include/endian.h b/include/endian.h index 1bd44451..c38c1ffc 100644 --- a/include/endian.h +++ b/include/endian.h @@ -24,17 +24,17 @@ static __inline uint16_t __bswap16(uint16_t __x) { - return __x<<8 | __x>>8; + return (__x << 8) | (__x >> 8); } static __inline uint32_t __bswap32(uint32_t __x) { - return __x>>24 | __x>>8&0xff00 | __x<<8&0xff0000 | __x<<24; + return (__x >> 24) | (__x>>8 & 0xff00) | (__x<<8 & 0xff0000) | (__x << 24); } static __inline uint64_t __bswap64(uint64_t __x) { - return __bswap32(__x)+0ULL<<32 | __bswap32(__x>>32); + return ((__bswap32(__x) + 0ULL) << 32) | __bswap32(__x >> 32); } #if __BYTE_ORDER == __LITTLE_ENDIAN diff --git a/src/ctype/towctrans.c b/src/ctype/towctrans.c index 8f681018..bd0136dd 100644 --- a/src/ctype/towctrans.c +++ b/src/ctype/towctrans.c @@ -259,12 +259,14 @@ static wchar_t __towcase(wchar_t wc, int lower) || (unsigned)wc - 0xabc0 <= 0xfeff-0xabc0) return wc; /* special case because the diff between upper/lower is too big */ - if (lower && (unsigned)wc - 0x10a0 < 0x2e) + if (lower && (unsigned)wc - 0x10a0 < 0x2e) { if (wc>0x10c5 && wc != 0x10c7 && wc != 0x10cd) return wc; else return wc + 0x2d00 - 0x10a0; - if (!lower && (unsigned)wc - 0x2d00 < 0x26) + } + if (!lower && (unsigned)wc - 0x2d00 < 0x26) { if (wc>0x2d25 && wc != 0x2d27 && wc != 0x2d2d) return wc; else return wc + 0x10a0 - 0x2d00; + } if (lower && (unsigned)wc - 0x13a0 < 0x50) return wc + 0xab70 - 0x13a0; if (!lower && (unsigned)wc - 0xab70 < 0x50) diff --git a/src/internal/shgetc.c b/src/internal/shgetc.c index ebd5fae7..c6aefed9 100644 --- a/src/internal/shgetc.c +++ b/src/internal/shgetc.c @@ -20,7 +20,7 @@ int __shgetc(FILE *f) { int c; off_t cnt = shcnt(f); - if (f->shlim && cnt >= f->shlim || (c=__uflow(f)) < 0) { + if ((f->shlim && cnt >= f->shlim) || (c=__uflow(f)) < 0) { f->shcnt = f->buf - f->rpos + cnt; f->shend = 0; return EOF; diff --git a/src/legacy/getpass.c b/src/legacy/getpass.c index d51286c0..036a7859 100644 --- a/src/legacy/getpass.c +++ b/src/legacy/getpass.c @@ -27,7 +27,7 @@ char *getpass(const char *prompt) l = read(fd, password, sizeof password); if (l >= 0) { - if (l > 0 && password[l-1] == '\n' || l==sizeof password) l--; + if ((l > 0 && password[l-1] == '\n') || l==sizeof password) l--; password[l] = 0; } diff --git a/src/locale/__mo_lookup.c b/src/locale/__mo_lookup.c index d18ab774..818b7f9b 100644 --- a/src/locale/__mo_lookup.c +++ b/src/locale/__mo_lookup.c @@ -3,7 +3,7 @@ static inline uint32_t swapc(uint32_t x, int c) { - return c ? x>>24 | x>>8&0xff00 | x<<8&0xff0000 | x<<24 : x; + return c ? (x>>24) | (x>>8 & 0xff00) | (x<<8 & 0xff0000) | (x<<24) : x; } const char *__mo_lookup(const void *p, size_t size, const char *s) diff --git a/src/locale/dcngettext.c b/src/locale/dcngettext.c index 8b891d00..30391ca6 100644 --- a/src/locale/dcngettext.c +++ b/src/locale/dcngettext.c @@ -176,7 +176,8 @@ notrans: snprintf(name, sizeof name, "%s/%.*s%.*s/%s/%s.mo\0", dirname, (int)loclen, locname, (int)alt_modlen, modname, catname, domainname); - if (map = __map_file(name, &map_size)) break; + map = __map_file(name, &map_size); + if (map) break; /* Try dropping @mod, _YY, then both. */ if (alt_modlen) { diff --git a/src/locale/iconv.c b/src/locale/iconv.c index 3047c27b..c0992833 100644 --- a/src/locale/iconv.c +++ b/src/locale/iconv.c @@ -181,7 +181,7 @@ static void put_16(unsigned char *s, unsigned c, int e) static unsigned get_32(const unsigned char *s, int e) { e &= 3; - return s[e]+0U<<24 | s[e^1]<<16 | s[e^2]<<8 | s[e^3]; + return (s[e]+0U)<<24 | s[e^1]<<16 | s[e^2]<<8 | s[e^3]; } static void put_32(unsigned char *s, unsigned c, int e) @@ -201,7 +201,7 @@ static unsigned legacy_map(const unsigned char *map, unsigned c) { if (c < 4*map[-1]) return c; unsigned x = c - 4*map[-1]; - x = map[x*5/4]>>2*x%8 | map[x*5/4+1]<<8-2*x%8 & 1023; + x = (map[x*5/4] >> 2*x%8) | ((map[x*5/4+1] << 8-2*x%8) & 1023); return x < 256 ? x : legacy_chars[x-256]; } @@ -495,7 +495,7 @@ size_t iconv(iconv_t cd, char **restrict in, size_t *restrict inb, char **restri if (c >= 93 || d >= 94) { c += (0xa1-0x81); d += 0xa1; - if (c >= 93 || c>=0xc6-0x81 && d>0x52) + if (c >= 93 || ((c >= 0xc6 - 0x81) && d>0x52)) goto ilseq; if (d-'A'<26) d = d-'A'; else if (d-'a'<26) d = d-'a'+26; diff --git a/src/locale/locale_map.c b/src/locale/locale_map.c index 2321bac0..8ee4641c 100644 --- a/src/locale/locale_map.c +++ b/src/locale/locale_map.c @@ -32,9 +32,9 @@ const struct __locale_map *__get_locale(int cat, const char *val) size_t l, n; if (!*val) { - (val = getenv("LC_ALL")) && *val || - (val = getenv(envvars[cat])) && *val || - (val = getenv("LANG")) && *val || + ((val = getenv("LC_ALL")) && *val) || + ((val = getenv(envvars[cat])) && *val) || + ((val = getenv("LANG")) && *val) || (val = "C.UTF-8"); } diff --git a/src/multibyte/c16rtomb.c b/src/multibyte/c16rtomb.c index 39ca3758..5ebfbc29 100644 --- a/src/multibyte/c16rtomb.c +++ b/src/multibyte/c16rtomb.c @@ -15,7 +15,7 @@ size_t c16rtomb(char *restrict s, char16_t c16, mbstate_t *restrict ps) } if (!*x && c16 - 0xd800u < 0x400) { - *x = c16 - 0xd7c0 << 10; + *x = (c16 - 0xd7c0) << 10; return 0; } diff --git a/src/network/dn_skipname.c b/src/network/dn_skipname.c index d54c2e5d..44f8e68c 100644 --- a/src/network/dn_skipname.c +++ b/src/network/dn_skipname.c @@ -3,10 +3,12 @@ int dn_skipname(const unsigned char *s, const unsigned char *end) { const unsigned char *p; - for (p=s; p=192) + else if (*p>=192) { if (p+1gr_name) - || !name && (*res)->gr_gid == gid) { + if ((name && !strcmp(name, (*res)->gr_name)) + || (!name && (*res)->gr_gid == gid)) { break; } } @@ -149,8 +149,8 @@ int __getgr_a(const char *name, gid_t gid, struct group *gr, char **buf, size_t goto cleanup_f; } - if (name && strcmp(name, gr->gr_name) - || !name && gid != gr->gr_gid) { + if ((name && strcmp(name, gr->gr_name)) + || (!name && gid != gr->gr_gid)) { rv = EIO; goto cleanup_f; } diff --git a/src/passwd/getpw_a.c b/src/passwd/getpw_a.c index 15a70c03..e1406319 100644 --- a/src/passwd/getpw_a.c +++ b/src/passwd/getpw_a.c @@ -34,8 +34,8 @@ int __getpw_a(const char *name, uid_t uid, struct passwd *pw, char **buf, size_t } while (!(rv = __getpwent_a(f, pw, buf, size, res)) && *res) { - if (name && !strcmp(name, (*res)->pw_name) - || !name && (*res)->pw_uid == uid) + if ((name && !strcmp(name, (*res)->pw_name)) + || (!name && (*res)->pw_uid == uid)) break; } fclose(f); @@ -122,8 +122,8 @@ int __getpw_a(const char *name, uid_t uid, struct passwd *pw, char **buf, size_t goto cleanup_f; } - if (name && strcmp(name, pw->pw_name) - || !name && uid != pw->pw_uid) { + if ((name && strcmp(name, pw->pw_name)) + || (!name && uid != pw->pw_uid)) { rv = EIO; goto cleanup_f; } diff --git a/src/prng/__rand48_step.c b/src/prng/__rand48_step.c index 94703d07..745fb0ee 100644 --- a/src/prng/__rand48_step.c +++ b/src/prng/__rand48_step.c @@ -4,8 +4,8 @@ uint64_t __rand48_step(unsigned short *xi, unsigned short *lc) { uint64_t a, x; - x = xi[0] | xi[1]+0U<<16 | xi[2]+0ULL<<32; - a = lc[0] | lc[1]+0U<<16 | lc[2]+0ULL<<32; + x = xi[0] | ((xi[1] + 0U) << 16) | ((xi[2] + 0ULL) << 32); + a = lc[0] | ((lc[1] + 0U) << 16) | ((lc[2] + 0ULL) << 32); x = a*x + lc[3]; xi[0] = x; xi[1] = x>>16; diff --git a/src/regex/glob.c b/src/regex/glob.c index aa1c6a44..72c9b5dc 100644 --- a/src/regex/glob.c +++ b/src/regex/glob.c @@ -142,7 +142,7 @@ static int do_glob(char *buf, size_t pos, int type, char *pat, int flags, int (* /* With GLOB_PERIOD, don't allow matching . or .. unless * fnmatch would match them with FNM_PERIOD rules in effect. */ if (p2 && (flags & GLOB_PERIOD) && de->d_name[0]=='.' - && (!de->d_name[1] || de->d_name[1]=='.' && !de->d_name[2]) + && (!de->d_name[1] || (de->d_name[1]=='.' && !de->d_name[2])) && fnmatch(pat, de->d_name, fnm_flags | FNM_PERIOD)) continue; diff --git a/src/select/select.c b/src/select/select.c index 02fd75c3..c913fe6b 100644 --- a/src/select/select.c +++ b/src/select/select.c @@ -16,7 +16,7 @@ int select(int n, fd_set *restrict rfds, fd_set *restrict wfds, fd_set *restrict return __syscall_ret(-EINVAL); time_t extra_secs = tv->tv_usec / 1000000; ts.tv_nsec = tv->tv_usec % 1000000 * 1000; - const time_t max_time = (1ULL<<8*sizeof(time_t)-1)-1; + const time_t max_time = (1ULL << (8 * sizeof(time_t) - 1)) - 1; ts.tv_sec = extra_secs > max_time - tv->tv_sec ? max_time : tv->tv_sec + extra_secs; } diff --git a/src/stdio/getc.h b/src/stdio/getc.h index e24f9905..7a4643cd 100644 --- a/src/stdio/getc.h +++ b/src/stdio/getc.h @@ -16,7 +16,7 @@ static int locking_getc(FILE *f) static inline int do_getc(FILE *f) { int l = f->lock; - if (l < 0 || l && (l & ~MAYBE_WAITERS) == __pthread_self()->tid) + if (l < 0 || (l && (l & ~MAYBE_WAITERS) == __pthread_self()->tid)) return getc_unlocked(f); return locking_getc(f); } diff --git a/src/stdio/putc.h b/src/stdio/putc.h index 2014c4ec..2d7dce16 100644 --- a/src/stdio/putc.h +++ b/src/stdio/putc.h @@ -16,7 +16,7 @@ static int locking_putc(int c, FILE *f) static inline int do_putc(int c, FILE *f) { int l = f->lock; - if (l < 0 || l && (l & ~MAYBE_WAITERS) == __pthread_self()->tid) + if (l < 0 || (l && (l & ~MAYBE_WAITERS) == __pthread_self()->tid)) return putc_unlocked(c, f); return locking_putc(c, f); } diff --git a/src/stdio/vfprintf.c b/src/stdio/vfprintf.c index 9b961e7f..695f8ed5 100644 --- a/src/stdio/vfprintf.c +++ b/src/stdio/vfprintf.c @@ -19,12 +19,12 @@ /* Convenient bit representation for modifier flags, which all fall * within 31 codepoints of the space character. */ -#define ALT_FORM (1U<<'#'-' ') -#define ZERO_PAD (1U<<'0'-' ') -#define LEFT_ADJ (1U<<'-'-' ') -#define PAD_POS (1U<<' '-' ') -#define MARK_POS (1U<<'+'-' ') -#define GROUPED (1U<<'\''-' ') +#define ALT_FORM (1U << ('#' - ' ')) +#define ZERO_PAD (1U << ('0' - ' ')) +#define LEFT_ADJ (1U << ('-' - ' ')) +#define PAD_POS (1U << (' ' - ' ')) +#define MARK_POS (1U << ('+' - ' ')) +#define GROUPED (1U << ('\'' - ' ')) #define FLAGMASK (ALT_FORM|ZERO_PAD|LEFT_ADJ|PAD_POS|MARK_POS|GROUPED) @@ -471,8 +471,8 @@ static int printf_core(FILE *f, const char *fmt, va_list *ap, union arg *nl_arg, } /* Read modifier flags */ - for (fl=0; (unsigned)*s-' '<32 && (FLAGMASK&(1U<<*s-' ')); s++) - fl |= 1U<<*s-' '; + for (fl=0; ((unsigned)*s-' ')<32 && (FLAGMASK&(1U<<(*s-' '))); s++) + fl |= 1U << (*s - ' '); /* Read field width */ if (*s=='*') { diff --git a/src/stdio/vfwprintf.c b/src/stdio/vfwprintf.c index 0adf0b7a..432f5e4a 100644 --- a/src/stdio/vfwprintf.c +++ b/src/stdio/vfwprintf.c @@ -12,12 +12,12 @@ /* Convenient bit representation for modifier flags, which all fall * within 31 codepoints of the space character. */ -#define ALT_FORM (1U<<'#'-' ') -#define ZERO_PAD (1U<<'0'-' ') -#define LEFT_ADJ (1U<<'-'-' ') -#define PAD_POS (1U<<' '-' ') -#define MARK_POS (1U<<'+'-' ') -#define GROUPED (1U<<'\''-' ') +#define ALT_FORM (1U << ('#' - ' ')) +#define ZERO_PAD (1U << ('0' - ' ')) +#define LEFT_ADJ (1U << ('-' - ' ')) +#define PAD_POS (1U << (' ' - ' ')) +#define MARK_POS (1U << ('+' - ' ')) +#define GROUPED (1U << ('\'' - ' ')) #define FLAGMASK (ALT_FORM|ZERO_PAD|LEFT_ADJ|PAD_POS|MARK_POS|GROUPED) @@ -184,8 +184,8 @@ static int wprintf_core(FILE *f, const wchar_t *fmt, va_list *ap, union arg *nl_ } /* Read modifier flags */ - for (fl=0; (unsigned)*s-' '<32 && (FLAGMASK&(1U<<*s-' ')); s++) - fl |= 1U<<*s-' '; + for (fl=0; ((unsigned)*s-' ')<32 && (FLAGMASK&(1U<<(*s-' '))); s++) + fl |= 1U << (*s - ' '); /* Read field width */ if (*s=='*') { diff --git a/src/thread/pthread_cancel.c b/src/thread/pthread_cancel.c index 2f9d5e97..9cc187f3 100644 --- a/src/thread/pthread_cancel.c +++ b/src/thread/pthread_cancel.c @@ -56,7 +56,7 @@ static void cancel_handler(int sig, siginfo_t *si, void *ctx) _sigaddset(&uc->uc_sigmask, SIGCANCEL); - if (self->cancelasync || pc >= (uintptr_t)__cp_begin && pc < (uintptr_t)__cp_end) { + if (self->cancelasync || (pc >= (uintptr_t)__cp_begin && pc < (uintptr_t)__cp_end)) { uc->uc_mcontext.MC_PC = (uintptr_t)__cp_cancel; #ifdef CANCEL_GOT uc->uc_mcontext.MC_GOT = CANCEL_GOT; --nextPart5961470.eLiFrnERWP Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part. Content-Transfer-Encoding: 7Bit -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEEEQbdvCCTWLgTlhzDuD1MjOuKv/YFAlvqF48ACgkQuD1MjOuK v/YUJA//Uedk+K4XtIqWSlgM6LX2U9SF+7/FjWN49y3NCd2O+cX4+QVz1ECAYbov YdyngnAmoxXCMv5KFzbMrN1Vom+Hm604g80x3Yvehx6Z8yuytzpQedBeas9I3vPj Wco/RiamHEbXZWKPm6iuv81z4sbvsG/KAnL/G/MSLkYP1Uwn/7JCQTf3Rz6Ny4q8 R78XnGqXt7rpJgizefw+RdIqInROCFQe0Ot8gsJ9S6reUZXGWlVqsnCDzcJeu9dl gD1IrCq9b/ehtyPZmc9pPoIJ6zrDi3iBxXnYpZx9yf6POkNtCg5Js1Dn39NYHpRY axRyGayhfWPaUJKqxp9WAcmpL/nsmpElwfHMjTgGu/ias5BK2GDnlP5RvM6NnoiA RW79ki1goEABZ8NOpWdE6Wf1DionfzJDeyxuRl5GJ1jxpC4Y+KB40vSyfkfN9ctK EVmZlQb7d1tLeo188ZIueCrxJEOAHXZYkBl6DXVXkfnUq7tminBAv0YgAGRjgS78 5LM6JG3IJT3qpG5UVeXO5fe85AKRvBkv8GBMu0LXjiwkoXBhEyr8VX2Kw8Prv8Hq HA8GlKFWbSh3h8KiERlCApxnoTu8OYWWmObvOUu3zcQBHEj8BzCqSo18RV4LVucI BH733CcDymZOqF5gkBepV5RiAx+e5YbWT/FJY8n5LlyZ728/Spo= =TZ/7 -----END PGP SIGNATURE----- --nextPart5961470.eLiFrnERWP--