[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1676444.u5bekTFY47@prometheus.sedrubal.de>
Date: Tue, 13 Nov 2018 01:15:11 +0100
From: Sedrubal <dev@...rubal.de>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] Fix many compiler warnings
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<end; p++)
+ for (p=s; p<end; p++) {
if (!*p) return p-s+1;
- else if (*p>=192)
+ else if (*p>=192) {
if (p+1<end) return p-s+2;
else break;
+ }
+ }
return -1;
}
diff --git a/src/passwd/getgr_a.c b/src/passwd/getgr_a.c
index afeb1ece..5605b4b6 100644
--- a/src/passwd/getgr_a.c
+++ b/src/passwd/getgr_a.c
@@ -33,8 +33,8 @@ int __getgr_a(const char *name, gid_t gid, struct group *gr,
char **buf, size_t
}
while (!(rv = __getgrent_a(f, gr, buf, size, mem, nmem, res)) && *res) {
- if (name && !strcmp(name, (*res)->gr_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;
Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
