From b0a365f74a0ac43fcbd53738844e577b2d9ec391 Mon Sep 17 00:00:00 2001 From: Denis Vlasenko Date: Mon, 1 Dec 2008 21:16:46 +0000 Subject: hostid: improve extremely unreadable parts *: remove checks of sigaction and sigprocmask results in cases where they clearly can't fail: sigaction(known_good_sig) sigprocmask(known_good_how) text data bss dec hex filename - 393 4 0 397 18d libc/pwd_grp/lckpwdf.o + 382 4 0 386 182 libc/pwd_grp/lckpwdf.o - 56 0 0 56 38 libc/signal/sigblock.o + 44 0 0 44 2c libc/signal/sigblock.o - 211 0 0 211 d3 libc/signal/sigset.o + 202 0 0 202 ca libc/signal/sigset.o - 56 0 0 56 38 libc/signal/sigsetmask.o + 44 0 0 44 2c libc/signal/sigsetmask.o - 309 0 0 309 135 libc/unistd/sleep.o + 256 0 0 256 100 libc/unistd/sleep.o --- libc/inet/hostid.c | 31 ++++++++++++++-------------- libc/pwd_grp/lckpwdf.c | 36 +++------------------------------ libc/signal/sigblock.c | 3 +-- libc/signal/sigintr.c | 1 + libc/signal/sigjmp.c | 2 +- libc/signal/signal.c | 1 + libc/signal/sigset.c | 8 +++----- libc/signal/sigsetmask.c | 3 +-- libc/signal/sigwait.c | 2 ++ libc/signal/sysv_signal.c | 1 + libc/stdlib/abort.c | 2 +- libc/sysdeps/linux/alpha/sigprocmask.c | 2 +- libc/sysdeps/linux/common/longjmp.c | 3 +-- libc/sysdeps/linux/common/sigprocmask.c | 4 ++-- libc/unistd/sleep.c | 27 ++++++++----------------- libpthread/linuxthreads.old/pthread.c | 3 +-- libpthread/linuxthreads/pthread.c | 3 +-- 17 files changed, 44 insertions(+), 88 deletions(-) diff --git a/libc/inet/hostid.c b/libc/inet/hostid.c index 42ee53fbd..cadcd1b28 100644 --- a/libc/inet/hostid.c +++ b/libc/inet/hostid.c @@ -33,11 +33,13 @@ int sethostid(long int new_id) int fd; int ret; - if (geteuid() || getuid()) return __set_errno(EPERM); - if ((fd=open(HOSTID,O_CREAT|O_WRONLY,0644))<0) return -1; - ret = write(fd,(void *)&new_id,sizeof(new_id)) == sizeof(new_id) - ? 0 : -1; - close (fd); + if (geteuid() || getuid()) + return __set_errno(EPERM); + fd = open(HOSTID, O_CREAT|O_WRONLY, 0644); + if (fd < 0) + return fd; + ret = write(fd, &new_id, sizeof(new_id)) == sizeof(new_id) ? 0 : -1; + close(fd); return ret; } #endif @@ -51,7 +53,8 @@ long int gethostid(void) * It is not an error if we cannot read this file. It is not even an * error if we cannot read all the bytes, we just carry on trying... */ - if ((fd=open(HOSTID,O_RDONLY))>=0 && read(fd,(void *)&id,sizeof(id))) + fd = open(HOSTID, O_RDONLY); + if (fd >= 0 && read(fd, &id, sizeof(id))) { close (fd); return id; @@ -69,7 +72,7 @@ long int gethostid(void) * setting one anyway. * Mitch */ - if (gethostname(host,MAXHOSTNAMELEN)>=0 && *host) { + if (gethostname(host, MAXHOSTNAMELEN) >= 0 && *host) { struct hostent *hp; struct in_addr in; struct hostent ghbn_h; @@ -84,21 +87,17 @@ long int gethostid(void) /*if ((hp = gethostbyname(host)) == (struct hostent *)NULL)*/ gethostbyname_r(host, &ghbn_h, ghbn_buf, sizeof(ghbn_buf), &hp, &ghbn_errno); - if (hp == (struct hostent *)NULL) - + if (hp == NULL) { /* This is not a error if we get here, as all it means is that * this host is not on a network and/or they have not * configured their network properly. So we return the unset * hostid which should be 0, meaning that they should set it !! */ return 0; - else { - memcpy((char *) &in, (char *) hp->h_addr, hp->h_length); - - /* Just so it doesn't look exactly like the IP addr */ - return(in.s_addr<<16|in.s_addr>>16); } + memcpy(&in, hp->h_addr, hp->h_length); + /* Just so it doesn't look exactly like the IP addr */ + return (in.s_addr<<16 | in.s_addr>>16); } - else return 0; - + return 0; } diff --git a/libc/pwd_grp/lckpwdf.c b/libc/pwd_grp/lckpwdf.c index 9b0b604e0..c0c8f0d90 100644 --- a/libc/pwd_grp/lckpwdf.c +++ b/libc/pwd_grp/lckpwdf.c @@ -80,25 +80,8 @@ lckpwdf (void) /* Make sure file gets correctly closed when process finished. */ flags = fcntl (lock_fd, F_GETFD); -#if 0 /* never fails */ - if (flags == -1) { - /* Cannot get file flags. */ - close(lock_fd); - lock_fd = -1; - goto DONE; - } -#endif flags |= FD_CLOEXEC; -#if 1 fcntl (lock_fd, F_SETFD, flags); -#else /* never fails */ - if (fcntl (lock_fd, F_SETFD, flags) < 0) { - /* Cannot set new flags. */ - close(lock_fd); - lock_fd = -1; - goto DONE; - } -#endif /* Now we have to get exclusive write access. Since multiple process could try this we won't stop when it first fails. Instead we set a timeout for the system call. Once the timer @@ -112,27 +95,14 @@ lckpwdf (void) new_act.sa_handler = noop_handler; __sigfillset (&new_act.sa_mask); - /* Install new action handler for alarm and save old. */ - if (sigaction (SIGALRM, &new_act, &saved_act) < 0) { - /* Cannot install signal handler. */ - close(lock_fd); - lock_fd = -1; - goto DONE; - } + /* Install new action handler for alarm and save old. + * This never fails in Linux. */ + sigaction (SIGALRM, &new_act, &saved_act); /* Now make sure the alarm signal is not blocked. */ __sigemptyset (&new_set); __sigaddset (&new_set, SIGALRM); -#if 1 sigprocmask (SIG_UNBLOCK, &new_set, &saved_set); -#else /* never fails */ - if (sigprocmask (SIG_UNBLOCK, &new_set, &saved_set) < 0) { - sigaction (SIGALRM, &saved_act, NULL); - close(lock_fd); - lock_fd = -1; - goto DONE; - } -#endif /* Start timer. If we cannot get the lock in the specified time we get a signal. */ diff --git a/libc/signal/sigblock.c b/libc/signal/sigblock.c index 72b42c45e..5a16f336b 100644 --- a/libc/signal/sigblock.c +++ b/libc/signal/sigblock.c @@ -32,8 +32,7 @@ int sigblock (int mask) sigset_t set, oset; sigset_set_old_mask (&set, mask); - if (sigprocmask (SIG_BLOCK, &set, &oset) < 0) - return -1; + sigprocmask (SIG_BLOCK, &set, &oset); /* can't fail */ return sigset_get_old_mask (&oset); } libc_hidden_def(sigblock) diff --git a/libc/signal/sigintr.c b/libc/signal/sigintr.c index 23f87e199..1a8d60eb2 100644 --- a/libc/signal/sigintr.c +++ b/libc/signal/sigintr.c @@ -34,6 +34,7 @@ int siginterrupt (int sig, int interrupt) #ifdef SA_RESTART struct sigaction action; + /* Fails if sig is bad. */ if (sigaction (sig, NULL, &action) < 0) return -1; diff --git a/libc/signal/sigjmp.c b/libc/signal/sigjmp.c index d83b49a0b..646949935 100644 --- a/libc/signal/sigjmp.c +++ b/libc/signal/sigjmp.c @@ -31,7 +31,7 @@ int __sigjmp_save (sigjmp_buf env, int savemask) attribute_hidden; int __sigjmp_save (sigjmp_buf env, int savemask) { env[0].__mask_was_saved = (savemask && - sigprocmask (SIG_BLOCK, (sigset_t *) NULL, &env[0].__saved_mask) == 0); + sigprocmask (SIG_BLOCK, NULL, &env[0].__saved_mask) == 0); return 0; } diff --git a/libc/signal/signal.c b/libc/signal/signal.c index 0ed281ab1..f3dfa33fc 100644 --- a/libc/signal/signal.c +++ b/libc/signal/signal.c @@ -45,6 +45,7 @@ __bsd_signal (int sig, __sighandler_t handler) __sigemptyset (&act.sa_mask); __sigaddset (&act.sa_mask, sig); act.sa_flags = __sigismember (&_sigintr, sig) ? 0 : SA_RESTART; + /* In Linux (as of 2.6.25), fails only if sig is SIGKILL or SIGSTOP */ if (sigaction (sig, &act, &oact) < 0) return SIG_ERR; diff --git a/libc/signal/sigset.c b/libc/signal/sigset.c index 91f433661..03f3dc869 100644 --- a/libc/signal/sigset.c +++ b/libc/signal/sigset.c @@ -42,13 +42,11 @@ __sighandler_t sigset (int sig, __sighandler_t disp) /* Handle SIG_HOLD first. */ if (disp == SIG_HOLD) { - /* Create an empty signal set. */ __sigemptyset (&set); __sigaddset (&set, sig); /* Add the signal set to the current signal mask. */ - if (sigprocmask (SIG_BLOCK, &set, NULL) < 0) - return SIG_ERR; + sigprocmask (SIG_BLOCK, &set, NULL); /* can't fail */ return SIG_HOLD; } @@ -56,6 +54,7 @@ __sighandler_t sigset (int sig, __sighandler_t disp) memset(&act, 0, sizeof(act)); act.sa_handler = disp; + /* In Linux (as of 2.6.25), fails only if sig is SIGKILL or SIGSTOP */ if (sigaction (sig, &act, &oact) < 0) return SIG_ERR; @@ -64,8 +63,7 @@ __sighandler_t sigset (int sig, __sighandler_t disp) __sigaddset (&set, sig); /* Remove the signal set from the current signal mask. */ - if (sigprocmask (SIG_UNBLOCK, &set, NULL) < 0) - return SIG_ERR; + sigprocmask (SIG_UNBLOCK, &set, NULL); /* can't fail */ return oact.sa_handler; } diff --git a/libc/signal/sigsetmask.c b/libc/signal/sigsetmask.c index 9349deb38..f8784e288 100644 --- a/libc/signal/sigsetmask.c +++ b/libc/signal/sigsetmask.c @@ -33,8 +33,7 @@ sigsetmask (int mask) sigset_t set, oset; sigset_set_old_mask (&set, mask); - if (sigprocmask (SIG_SETMASK, &set, &oset) < 0) - return -1; + sigprocmask (SIG_SETMASK, &set, &oset); /* can't fail */ return sigset_get_old_mask (&oset); } libc_hidden_def(sigsetmask) diff --git a/libc/signal/sigwait.c b/libc/signal/sigwait.c index 64a889f7b..bed7f979a 100644 --- a/libc/signal/sigwait.c +++ b/libc/signal/sigwait.c @@ -74,6 +74,8 @@ int __sigwait (const sigset_t *set, int *sig) __sigdelset (&tmp_mask, this); /* Register temporary action handler. */ + /* In Linux (as of 2.6.25), fails only if sig is SIGKILL or SIGSTOP */ + /* (so, will it work correctly if set has, say, SIGSTOP?) */ if (sigaction (this, &action, &saved[this]) != 0) goto restore_handler; } diff --git a/libc/signal/sysv_signal.c b/libc/signal/sysv_signal.c index f573482f9..118094b27 100644 --- a/libc/signal/sysv_signal.c +++ b/libc/signal/sysv_signal.c @@ -49,6 +49,7 @@ __sighandler_t __sysv_signal (int sig, __sighandler_t handler) act.sa_handler = handler; __sigemptyset (&act.sa_mask); act.sa_flags = (SA_ONESHOT | SA_NOMASK | SA_INTERRUPT) & ~SA_RESTART; + /* In Linux (as of 2.6.25), fails only if sig is SIGKILL or SIGSTOP */ if (sigaction (sig, &act, &oact) < 0) return SIG_ERR; diff --git a/libc/stdlib/abort.c b/libc/stdlib/abort.c index 9f0fe54b8..fcf90187c 100644 --- a/libc/stdlib/abort.c +++ b/libc/stdlib/abort.c @@ -62,7 +62,7 @@ void abort(void) /* Unmask SIGABRT to be sure we can get it */ __sigemptyset(&sigs); __sigaddset(&sigs, SIGABRT); - sigprocmask(SIG_UNBLOCK, &sigs, (sigset_t *) NULL); + sigprocmask(SIG_UNBLOCK, &sigs, NULL); while (1) { /* Try to suicide with a SIGABRT */ diff --git a/libc/sysdeps/linux/alpha/sigprocmask.c b/libc/sysdeps/linux/alpha/sigprocmask.c index e143f61e3..832684cef 100644 --- a/libc/sysdeps/linux/alpha/sigprocmask.c +++ b/libc/sysdeps/linux/alpha/sigprocmask.c @@ -44,7 +44,7 @@ sigprocmask (int how, const sigset_t *set, sigset_t *oset) result = osf_sigprocmask(how, setval); if (result == -1) - /* If there are ever more than 63 signals, we need to recode this + /* If there are ever more than 64 signals, we need to recode this in assembler since we wouldn't be able to distinguish a mask of all 1s from -1, but for now, we're doing just fine... */ return result; diff --git a/libc/sysdeps/linux/common/longjmp.c b/libc/sysdeps/linux/common/longjmp.c index 672754a73..e600c7f13 100644 --- a/libc/sysdeps/linux/common/longjmp.c +++ b/libc/sysdeps/linux/common/longjmp.c @@ -38,8 +38,7 @@ void __libc_longjmp (sigjmp_buf env, int val) if (env[0].__mask_was_saved) /* Restore the saved signal mask. */ - (void) sigprocmask (SIG_SETMASK, &env[0].__saved_mask, - (sigset_t *) NULL); + (void) sigprocmask (SIG_SETMASK, &env[0].__saved_mask, NULL); /* Call the machine-dependent function to restore machine state. */ __longjmp (env[0].__jmpbuf, val ?: 1); diff --git a/libc/sysdeps/linux/common/sigprocmask.c b/libc/sysdeps/linux/common/sigprocmask.c index a9a913e10..febdca014 100644 --- a/libc/sysdeps/linux/common/sigprocmask.c +++ b/libc/sysdeps/linux/common/sigprocmask.c @@ -28,7 +28,7 @@ int sigprocmask(int how, const sigset_t * set, sigset_t * oldset) if (set && # if (SIG_BLOCK == 0) && (SIG_UNBLOCK == 1) && (SIG_SETMASK == 2) (((unsigned int) how) > 2) -#elif (SIG_BLOCK == 1) && (SIG_UNBLOCK == 2) && (SIG_SETMASK == 3) +# elif (SIG_BLOCK == 1) && (SIG_UNBLOCK == 2) && (SIG_SETMASK == 3) (((unsigned int)(how-1)) > 2) # else # warning "compile time assumption violated.. slow path..." @@ -55,7 +55,7 @@ int sigprocmask(int how, const sigset_t * set, sigset_t * oldset) if (set && # if (SIG_BLOCK == 0) && (SIG_UNBLOCK == 1) && (SIG_SETMASK == 2) (((unsigned int) how) > 2) -#elif (SIG_BLOCK == 1) && (SIG_UNBLOCK == 2) && (SIG_SETMASK == 3) +# elif (SIG_BLOCK == 1) && (SIG_UNBLOCK == 2) && (SIG_SETMASK == 3) (((unsigned int)(how-1)) > 2) # else # warning "compile time assumption violated.. slow path..." diff --git a/libc/unistd/sleep.c b/libc/unistd/sleep.c index 8cac306ce..4d55d843d 100644 --- a/libc/unistd/sleep.c +++ b/libc/unistd/sleep.c @@ -68,8 +68,7 @@ unsigned int sleep (unsigned int seconds) in libc. We block SIGCHLD first. */ __sigemptyset (&set); __sigaddset (&set, SIGCHLD); - if (sigprocmask (SIG_BLOCK, &set, &oset)) - return -1; + sigprocmask (SIG_BLOCK, &set, &oset); /* can't fail */ /* If SIGCHLD is already blocked, we don't have to do anything. */ if (!__sigismember (&oset, SIGCHLD)) @@ -80,15 +79,7 @@ unsigned int sleep (unsigned int seconds) __sigemptyset (&set); __sigaddset (&set, SIGCHLD); - /* We get the signal handler for SIGCHLD. */ - if (sigaction (SIGCHLD, (struct sigaction *) NULL, &oact) < 0) - { - saved_errno = errno; - /* Restore the original signal mask. */ - (void) sigprocmask (SIG_SETMASK, &oset, (sigset_t *) NULL); - __set_errno (saved_errno); - return -1; - } + sigaction (SIGCHLD, NULL, &oact); /* never fails */ if (oact.sa_handler == SIG_IGN) { @@ -97,13 +88,13 @@ unsigned int sleep (unsigned int seconds) saved_errno = errno; /* Restore the original signal mask. */ - (void) sigprocmask (SIG_SETMASK, &oset, (sigset_t *) NULL); + sigprocmask (SIG_SETMASK, &oset, NULL); __set_errno (saved_errno); } else { /* We should unblock SIGCHLD. Restore the original signal mask. */ - (void) sigprocmask (SIG_SETMASK, &oset, (sigset_t *) NULL); + sigprocmask (SIG_SETMASK, &oset, NULL); result = nanosleep (&ts, &ts); } } @@ -138,27 +129,25 @@ unsigned int sleep (unsigned int seconds) /* block SIGALRM */ __sigemptyset (&set); __sigaddset (&set, SIGALRM); - if (sigprocmask (SIG_BLOCK, &set, &oset)) - return seconds; + sigprocmask (SIG_BLOCK, &set, &oset); /* can't fail */ act.sa_handler = sleep_alarm_handler; act.sa_flags = 0; act.sa_mask = oset; - if (sigaction(SIGALRM, &act, &oact) < 0) - return seconds; + sigaction(SIGALRM, &act, &oact); /* never fails */ before = time(NULL); remaining = alarm(seconds); if (remaining && remaining > seconds) { /* restore user's alarm */ - (void) sigaction(SIGALRM, &oact, (struct sigaction *) NULL); + sigaction(SIGALRM, &oact, NULL); alarm(remaining); /* restore old alarm */ sigsuspend(&oset); after = time(NULL); } else { sigsuspend (&oset); after = time(NULL); - (void) sigaction (SIGALRM, &oact, NULL); + sigaction (SIGALRM, &oact, NULL); } result = after - before; alarm(remaining > result ? remaining - result : 0); diff --git a/libpthread/linuxthreads.old/pthread.c b/libpthread/linuxthreads.old/pthread.c index 391527e62..562aa433c 100644 --- a/libpthread/linuxthreads.old/pthread.c +++ b/libpthread/linuxthreads.old/pthread.c @@ -650,8 +650,7 @@ int pthread_create(pthread_t *thread, const pthread_attr_t *attr, request.req_args.create.attr = attr; request.req_args.create.fn = start_routine; request.req_args.create.arg = arg; - sigprocmask(SIG_SETMASK, (const sigset_t *) NULL, - &request.req_args.create.mask); + sigprocmask(SIG_SETMASK, NULL, &request.req_args.create.mask); PDEBUG("write REQ_CREATE to manager thread\n"); TEMP_FAILURE_RETRY(__libc_write(__pthread_manager_request, (char *) &request, sizeof(request))); diff --git a/libpthread/linuxthreads/pthread.c b/libpthread/linuxthreads/pthread.c index 7f4f86b46..2d0f49ee9 100644 --- a/libpthread/linuxthreads/pthread.c +++ b/libpthread/linuxthreads/pthread.c @@ -831,8 +831,7 @@ int __pthread_create(pthread_t *thread, const pthread_attr_t *attr, request.req_args.create.attr = attr; request.req_args.create.fn = start_routine; request.req_args.create.arg = arg; - sigprocmask(SIG_SETMASK, (const sigset_t *) NULL, - &request.req_args.create.mask); + sigprocmask(SIG_SETMASK, NULL, &request.req_args.create.mask); TEMP_FAILURE_RETRY(write_not_cancel(__pthread_manager_request, (char *) &request, sizeof(request))); suspend(self); -- cgit v1.2.3