Hi Waldemar,
On Wed, Jan 25, 2017 at 07:02:53AM +0100, Waldemar Brodkorb wrote:
> I would like to suggest following changeset, any opinions?
>
> Follow GNU C Library from c579f48edba88380635ab98cb612030e3ed8691e
> and remove the PID caching. These simplifies the architecture specific
> assembly code.
> The run of the test suite found no regressions, it even solves
> some of the test failures for x86/x86_64/sparc.
>
> Signed-off-by: Waldemar Brodkorb <[email protected]>
> ---
Looks good to me, but there's a couple of questions below.
[...]
> diff --git a/libpthread/nptl/descr.h b/libpthread/nptl/descr.h
> index 3fda681..30a4f4b 100644
> --- a/libpthread/nptl/descr.h
> +++ b/libpthread/nptl/descr.h
> @@ -153,7 +153,7 @@ struct pthread
> pid_t tid;
>
> /* Process ID - thread group ID in kernel speak. */
> - pid_t pid;
> + pid_t pid_unused;
Why is it left here?
> /* List of robust mutexes the thread is holding. */
> #ifdef __PTHREAD_MUTEX_HAVE_PREV
[...]
> diff --git a/libpthread/nptl/sysdeps/unix/sysv/linux/raise.c
> b/libpthread/nptl/sysdeps/unix/sysv/linux/raise.c
> index 6b15938..b168c15 100644
> --- a/libpthread/nptl/sysdeps/unix/sysv/linux/raise.c
> +++ b/libpthread/nptl/sysdeps/unix/sysv/linux/raise.c
> @@ -26,31 +26,27 @@
> int
> raise (int sig)
> {
> - struct pthread *pd = THREAD_SELF;
> - pid_t pid = THREAD_GETMEM (pd, pid);
> - pid_t selftid = THREAD_GETMEM (pd, tid);
> - if (selftid == 0)
> - {
> - /* This system call is not supposed to fail. */
> -#ifdef INTERNAL_SYSCALL
> - INTERNAL_SYSCALL_DECL (err);
> - selftid = INTERNAL_SYSCALL (gettid, err, 0);
> -#else
> - selftid = INLINE_SYSCALL (gettid, 0);
> -#endif
> - THREAD_SETMEM (pd, tid, selftid);
> -
> - /* We do not set the PID field in the TID here since we might be
> - called from a signal handler while the thread executes fork. */
> - pid = selftid;
> - }
> - else
> - /* raise is an async-safe function. It could be called while the
> - fork/vfork function temporarily invalidated the PID field. Adjust for
> - that. */
> - if (__builtin_expect (pid <= 0, 0))
> - pid = (pid & INT_MAX) == 0 ? selftid : -pid;
> -
> - return INLINE_SYSCALL (tgkill, 3, pid, selftid, sig);
> + /* rt_sigprocmask may fail if:
> +
> + 1. sigsetsize != sizeof (sigset_t) (EINVAL)
> + 2. a failure in copy from/to user space (EFAULT)
> + 3. an invalid 'how' operation (EINVAL)
> +
> + The first case is already handle in glibc syscall call by using the arch
> + defined _NSIG. Second case is handled by using a stack allocated mask.
> + The last one should be handled by the block/unblock functions. */
> +
> + //sigset_t set;
> + //__libc_signal_block_app (&set);
Why adding commented out code instead of using e.g. sigprocmask?
> +
> + INTERNAL_SYSCALL_DECL (err);
> + pid_t pid = INTERNAL_SYSCALL (getpid, err, 0);
> + pid_t tid = INTERNAL_SYSCALL (gettid, err, 0);
> +
> + int ret = INLINE_SYSCALL (tgkill, 3, pid, tid, sig);
> +
> + //__libc_signal_restore_set (&set);
> +
> + return ret;
> }
> libc_hidden_def (raise)
--
Thanks.
-- Max
_______________________________________________
devel mailing list
[email protected]
http://mailman.uclibc-ng.org/cgi-bin/mailman/listinfo/devel
