* [PATCH 0/2] Resolve compiler warnings in master @ 2019-06-29 21:22 Samuel Holland 2019-06-29 21:22 ` [PATCH 1/2] resolve -Wrestrict warnings Samuel Holland ` (2 more replies) 0 siblings, 3 replies; 6+ messages in thread From: Samuel Holland @ 2019-06-29 21:22 UTC (permalink / raw) To: musl; +Cc: Samuel Holland These two patches resolve some compiler warnings about mismatched attributes and restrict violations. There's another warning, related to duplicate definitions of TIOCSER_TEMT on some arches; I'm not sure which header needs to be changed. It results in: ./include/sys/ioctl.h:47: warning: "TIOCSER_TEMT" redefined The definitions are: arch/mips/bits/termios.h:#define TIOCSER_TEMT 0x01 arch/mips64/bits/termios.h:#define TIOCSER_TEMT 0x01 arch/mipsn32/bits/termios.h:#define TIOCSER_TEMT 0x01 arch/powerpc/bits/termios.h:#define TIOCSER_TEMT 0x01 arch/powerpc64/bits/termios.h:#define TIOCSER_TEMT 0x01 include/sys/ioctl.h:#define TIOCSER_TEMT 1 Samuel Holland (2): resolve -Wrestrict warnings use the correct attributes for ___errno_location src/aio/lio_listio.c | 6 +++--- src/errno/__errno_location.c | 6 +++++- src/signal/sigset.c | 8 ++++---- src/unistd/ualarm.c | 6 +++--- 4 files changed, 15 insertions(+), 11 deletions(-) -- 2.21.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] resolve -Wrestrict warnings 2019-06-29 21:22 [PATCH 0/2] Resolve compiler warnings in master Samuel Holland @ 2019-06-29 21:22 ` Samuel Holland 2019-06-29 21:22 ` [PATCH 2/2] use the correct attributes for ___errno_location Samuel Holland 2019-06-29 21:46 ` [PATCH 0/2] Resolve compiler warnings in master Rich Felker 2 siblings, 0 replies; 6+ messages in thread From: Samuel Holland @ 2019-06-29 21:22 UTC (permalink / raw) To: musl; +Cc: Samuel Holland The old/new parameters to pthread_sigmask, sigprocmask, and setitimer are marked __restrict, so passing the same address to both is prohibited. Modify callers of these functions to use a separate object for each argument. --- src/aio/lio_listio.c | 6 +++--- src/signal/sigset.c | 8 ++++---- src/unistd/ualarm.c | 6 +++--- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/aio/lio_listio.c b/src/aio/lio_listio.c index 7b6a03d3..0799c15d 100644 --- a/src/aio/lio_listio.c +++ b/src/aio/lio_listio.c @@ -113,7 +113,7 @@ int lio_listio(int mode, struct aiocb *restrict const *restrict cbs, int cnt, st if (st) { pthread_attr_t a; - sigset_t set; + sigset_t set, set_old; pthread_t td; if (sev->sigev_notify == SIGEV_THREAD) { @@ -128,13 +128,13 @@ int lio_listio(int mode, struct aiocb *restrict const *restrict cbs, int cnt, st } pthread_attr_setdetachstate(&a, PTHREAD_CREATE_DETACHED); sigfillset(&set); - pthread_sigmask(SIG_BLOCK, &set, &set); + pthread_sigmask(SIG_BLOCK, &set, &set_old); if (pthread_create(&td, &a, wait_thread, st)) { free(st); errno = EAGAIN; return -1; } - pthread_sigmask(SIG_SETMASK, &set, 0); + pthread_sigmask(SIG_SETMASK, &set_old, 0); } return 0; diff --git a/src/signal/sigset.c b/src/signal/sigset.c index 0d7b4564..f3e8c407 100644 --- a/src/signal/sigset.c +++ b/src/signal/sigset.c @@ -3,7 +3,7 @@ void (*sigset(int sig, void (*handler)(int)))(int) { struct sigaction sa, sa_old; - sigset_t mask; + sigset_t mask, mask_old; sigemptyset(&mask); if (sigaddset(&mask, sig) < 0) @@ -12,7 +12,7 @@ void (*sigset(int sig, void (*handler)(int)))(int) if (handler == SIG_HOLD) { if (sigaction(sig, 0, &sa_old) < 0) return SIG_ERR; - if (sigprocmask(SIG_BLOCK, &mask, &mask) < 0) + if (sigprocmask(SIG_BLOCK, &mask, &mask_old) < 0) return SIG_ERR; } else { sa.sa_handler = handler; @@ -20,8 +20,8 @@ void (*sigset(int sig, void (*handler)(int)))(int) sigemptyset(&sa.sa_mask); if (sigaction(sig, &sa, &sa_old) < 0) return SIG_ERR; - if (sigprocmask(SIG_UNBLOCK, &mask, &mask) < 0) + if (sigprocmask(SIG_UNBLOCK, &mask, &mask_old) < 0) return SIG_ERR; } - return sigismember(&mask, sig) ? SIG_HOLD : sa_old.sa_handler; + return sigismember(&mask_old, sig) ? SIG_HOLD : sa_old.sa_handler; } diff --git a/src/unistd/ualarm.c b/src/unistd/ualarm.c index 855504bc..2985855c 100644 --- a/src/unistd/ualarm.c +++ b/src/unistd/ualarm.c @@ -7,7 +7,7 @@ unsigned ualarm(unsigned value, unsigned interval) struct itimerval it = { .it_interval.tv_usec = interval, .it_value.tv_usec = value - }; - setitimer(ITIMER_REAL, &it, &it); - return it.it_value.tv_sec*1000000 + it.it_value.tv_usec; + }, it_old; + setitimer(ITIMER_REAL, &it, &it_old); + return it_old.it_value.tv_sec*1000000 + it_old.it_value.tv_usec; } -- 2.21.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] use the correct attributes for ___errno_location 2019-06-29 21:22 [PATCH 0/2] Resolve compiler warnings in master Samuel Holland 2019-06-29 21:22 ` [PATCH 1/2] resolve -Wrestrict warnings Samuel Holland @ 2019-06-29 21:22 ` Samuel Holland 2019-06-29 21:49 ` Rich Felker 2019-06-29 21:46 ` [PATCH 0/2] Resolve compiler warnings in master Rich Felker 2 siblings, 1 reply; 6+ messages in thread From: Samuel Holland @ 2019-06-29 21:22 UTC (permalink / raw) To: musl; +Cc: Samuel Holland In the public header, __errno_location is declared with the "const" attribute, conditional on __GNUC__. Ensure that its internal alias has the same attributes. --- src/errno/__errno_location.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/errno/__errno_location.c b/src/errno/__errno_location.c index 7f9d6027..b59919c3 100644 --- a/src/errno/__errno_location.c +++ b/src/errno/__errno_location.c @@ -6,4 +6,8 @@ int *__errno_location(void) return &__pthread_self()->errno_val; } -weak_alias(__errno_location, ___errno_location); +weak_alias(__errno_location, ___errno_location) +#ifdef __GNUC__ +__attribute__((const)) +#endif +; -- 2.21.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] use the correct attributes for ___errno_location 2019-06-29 21:22 ` [PATCH 2/2] use the correct attributes for ___errno_location Samuel Holland @ 2019-06-29 21:49 ` Rich Felker 0 siblings, 0 replies; 6+ messages in thread From: Rich Felker @ 2019-06-29 21:49 UTC (permalink / raw) To: musl On Sat, Jun 29, 2019 at 04:22:44PM -0500, Samuel Holland wrote: > In the public header, __errno_location is declared with the "const" > attribute, conditional on __GNUC__. Ensure that its internal alias has > the same attributes. > --- > src/errno/__errno_location.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/src/errno/__errno_location.c b/src/errno/__errno_location.c > index 7f9d6027..b59919c3 100644 > --- a/src/errno/__errno_location.c > +++ b/src/errno/__errno_location.c > @@ -6,4 +6,8 @@ int *__errno_location(void) > return &__pthread_self()->errno_val; > } > > -weak_alias(__errno_location, ___errno_location); > +weak_alias(__errno_location, ___errno_location) > +#ifdef __GNUC__ > +__attribute__((const)) > +#endif > +; > -- > 2.21.0 Thanks for catching this. It's probably a significant regression in codegen. I think the attribute should be on the declaration in src/include/errno.h though, not on the weak_alias definition. Most importantly this is needed for it to affect codegen in the callers. But it's also for consistency with the approach of having attributes on the declarations rather than the definitions (to ensure everyone gets a consistent view of them), and to avoid assumptions about what the weak_alias macro expands to. Rich ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] Resolve compiler warnings in master 2019-06-29 21:22 [PATCH 0/2] Resolve compiler warnings in master Samuel Holland 2019-06-29 21:22 ` [PATCH 1/2] resolve -Wrestrict warnings Samuel Holland 2019-06-29 21:22 ` [PATCH 2/2] use the correct attributes for ___errno_location Samuel Holland @ 2019-06-29 21:46 ` Rich Felker 2019-06-29 23:05 ` Samuel Holland 2 siblings, 1 reply; 6+ messages in thread From: Rich Felker @ 2019-06-29 21:46 UTC (permalink / raw) To: musl On Sat, Jun 29, 2019 at 04:22:42PM -0500, Samuel Holland wrote: > These two patches resolve some compiler warnings about mismatched > attributes and restrict violations. There's another warning, related > to duplicate definitions of TIOCSER_TEMT on some arches; I'm not sure > which header needs to be changed. It results in: > > ../include/sys/ioctl.h:47: warning: "TIOCSER_TEMT" redefined > > The definitions are: > > arch/mips/bits/termios.h:#define TIOCSER_TEMT 0x01 > arch/mips64/bits/termios.h:#define TIOCSER_TEMT 0x01 > arch/mipsn32/bits/termios.h:#define TIOCSER_TEMT 0x01 > arch/powerpc/bits/termios.h:#define TIOCSER_TEMT 0x01 > arch/powerpc64/bits/termios.h:#define TIOCSER_TEMT 0x01 > include/sys/ioctl.h:#define TIOCSER_TEMT 1 I don't see how this is happening with a consistent tree. Commit 3517d74a5e04a377192d1f4882ad6c8dc22ce69a removed it from the bits headers. Rich ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] Resolve compiler warnings in master 2019-06-29 21:46 ` [PATCH 0/2] Resolve compiler warnings in master Rich Felker @ 2019-06-29 23:05 ` Samuel Holland 0 siblings, 0 replies; 6+ messages in thread From: Samuel Holland @ 2019-06-29 23:05 UTC (permalink / raw) To: musl, Rich Felker On 6/29/19 4:46 PM, Rich Felker wrote: > On Sat, Jun 29, 2019 at 04:22:42PM -0500, Samuel Holland wrote: >> These two patches resolve some compiler warnings about mismatched >> attributes and restrict violations. There's another warning, related >> to duplicate definitions of TIOCSER_TEMT on some arches; I'm not sure >> which header needs to be changed. It results in: >> >> ../include/sys/ioctl.h:47: warning: "TIOCSER_TEMT" redefined >> >> The definitions are: >> >> arch/mips/bits/termios.h:#define TIOCSER_TEMT 0x01 >> arch/mips64/bits/termios.h:#define TIOCSER_TEMT 0x01 >> arch/mipsn32/bits/termios.h:#define TIOCSER_TEMT 0x01 >> arch/powerpc/bits/termios.h:#define TIOCSER_TEMT 0x01 >> arch/powerpc64/bits/termios.h:#define TIOCSER_TEMT 0x01 >> include/sys/ioctl.h:#define TIOCSER_TEMT 1 > > I don't see how this is happening with a consistent tree. Commit > 3517d74a5e04a377192d1f4882ad6c8dc22ce69a removed it from the bits > headers. That commit removed it from bits/ioctl.h. The definition is still in bits/termios.h, where it was changed in 9eda4dc69c33852c97c6f69176bf45ffc80b522f to match the old value in bits/ioctl.h. So maybe sys/ioctl.h needs to be changed from 1 to 0x01? Cheers, Samuel ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-06-29 23:05 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-06-29 21:22 [PATCH 0/2] Resolve compiler warnings in master Samuel Holland 2019-06-29 21:22 ` [PATCH 1/2] resolve -Wrestrict warnings Samuel Holland 2019-06-29 21:22 ` [PATCH 2/2] use the correct attributes for ___errno_location Samuel Holland 2019-06-29 21:49 ` Rich Felker 2019-06-29 21:46 ` [PATCH 0/2] Resolve compiler warnings in master Rich Felker 2019-06-29 23:05 ` Samuel Holland
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/musl/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).