From patchwork Tue Oct 13 19:05:53 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "H.J. Lu" X-Patchwork-Id: 529904 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3BE8814016A for ; Wed, 14 Oct 2015 06:06:04 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.b=ViH09zYM; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; q=dns; s=default; b= gVSjkLhyWtT6QjxYDtoT/Y8FSC2K0goqyP1nfLvMgyRixjOXBTSabrujjX7SvFVh S428ccwqIvK/oGZ26YSHtyydlxpN01XE7OBtgMqrLMrFTCL+wjrDFBqoFBZoJOLR oNrGhMo4ypj5oN4lnCl+kLzYruyMpBhNPfTXttQnNP0= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; s=default; bh=Sz4Vm cm1ppncC3bXLKbtGTFh0MQ=; b=ViH09zYMCcobZKDqXmtdeaUHSrbHoXj+0QuX+ 1avvWtGoJpGthIRytUFcbxT/EKHiPgQEZjQQevUPIX4H8ApovET8sX0XYkZphQnr dLmv+f6yeB04jGF3WafMUY3MvWZS2aY37h6M7c26KqwUev/BZJm/gm8JZPszKPoy dAML1s= Received: (qmail 63747 invoked by alias); 13 Oct 2015 19:05:59 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 63738 invoked by uid 89); 13 Oct 2015 19:05:59 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=1.3 required=5.0 tests=AWL, BAYES_99, BAYES_999, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=no version=3.3.2 X-HELO: mail-ob0-f174.google.com MIME-Version: 1.0 X-Received: by 10.182.96.197 with SMTP id du5mr9852254obb.18.1444763153795; Tue, 13 Oct 2015 12:05:53 -0700 (PDT) In-Reply-To: <20151013134618.GA21180@gmail.com> References: <20151012232011.GB8797@intel.com> <20151013134618.GA21180@gmail.com> Date: Tue, 13 Oct 2015 12:05:53 -0700 Message-ID: Subject: Re: [PATCH 4/6] Use INTERNAL_SYSCALL and INLINE_SYSCALL_ERROR_RETURN From: "H.J. Lu" To: Andreas Schwab Cc: GNU C Library On Tue, Oct 13, 2015 at 6:46 AM, H.J. Lu wrote: > On Tue, Oct 13, 2015 at 09:37:48AM +0200, Andreas Schwab wrote: >> "H.J. Lu" writes: >> >> > + INTERNAL_SYSCALL_DECL (err); >> > + result = INTERNAL_SYSCALL (fstat64, err, 2, fd, &buf64); >> > + if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (result, ))) >> >> Either use err in both macro calls, or omit it in both. Our API calls >> for the use of it, so I prefer the former. >> > > Here is the updated patch. > First 2 patches have been checked into master. Here is the updated patch. There is no code change in libc.so on x86-64. OK for master? Thanks. From 958f78a23b3020a0382f2a4b5c000899c7b8bb2a Mon Sep 17 00:00:00 2001 From: "H.J. Lu" Date: Mon, 14 Sep 2015 07:00:59 -0700 Subject: [PATCH 4/6] Use INTERNAL_SYSCALL and INLINE_SYSCALL_ERROR_RETURN_VALUE This patch uses INTERNAL_SYSCALL and INLINE_SYSCALL_ERROR_RETURN_VALUE to avoid reading and writing errno directly so that we don't need to call __x86.get_pc_thunk.reg to load PC into reg in case there is an error. * sysdeps/unix/sysv/linux/i386/brk.c (__brk): Use INLINE_SYSCALL_ERROR_RETURN_VALUE. * sysdeps/unix/sysv/linux/i386/fxstatat.c (__fxstatat): Likewise. * sysdeps/unix/sysv/linux/i386/setegid.c (setegid): Likewise. * sysdeps/unix/sysv/linux/i386/seteuid.c (seteuid): Likewise. * sysdeps/unix/sysv/linux/i386/fxstat.c (__fxstat): Use INTERNAL_SYSCALLINTERNAL_SYSCALL and INLINE_SYSCALL_ERROR_RETURN_VALUE. * sysdeps/unix/sysv/linux/i386/lockf64.c (lockf64): Likewise. * sysdeps/unix/sysv/linux/i386/lxstat.c (__lxstat): Likewise. * sysdeps/unix/sysv/linux/i386/sigaction.c (__libc_sigaction): Likewise. * sysdeps/unix/sysv/linux/i386/xstat.c (__xstat): Likewise. --- sysdeps/unix/sysv/linux/i386/brk.c | 12 ++---------- sysdeps/unix/sysv/linux/i386/fxstat.c | 10 ++++++---- sysdeps/unix/sysv/linux/i386/fxstatat.c | 9 +++------ sysdeps/unix/sysv/linux/i386/lockf64.c | 13 +++++++------ sysdeps/unix/sysv/linux/i386/lxstat.c | 10 ++++++---- sysdeps/unix/sysv/linux/i386/setegid.c | 5 +---- sysdeps/unix/sysv/linux/i386/seteuid.c | 5 +---- sysdeps/unix/sysv/linux/i386/sigaction.c | 12 +++++++----- sysdeps/unix/sysv/linux/i386/xstat.c | 10 ++++++---- 9 files changed, 39 insertions(+), 47 deletions(-) diff --git a/sysdeps/unix/sysv/linux/i386/brk.c b/sysdeps/unix/sysv/linux/i386/brk.c index 5b9a0ce..e360664 100644 --- a/sysdeps/unix/sysv/linux/i386/brk.c +++ b/sysdeps/unix/sysv/linux/i386/brk.c @@ -31,19 +31,11 @@ weak_alias (__curbrk, ___brk_addr) int __brk (void *addr) { - void *newbrk; - INTERNAL_SYSCALL_DECL (err); - newbrk = (void *) INTERNAL_SYSCALL (brk, err, 1, addr); - + void *newbrk = (void *) INTERNAL_SYSCALL (brk, err, 1, addr); __curbrk = newbrk; - if (newbrk < addr) - { - __set_errno (ENOMEM); - return -1; - } - + return INLINE_SYSCALL_ERROR_RETURN_VALUE (ENOMEM); return 0; } weak_alias (__brk, brk) diff --git a/sysdeps/unix/sysv/linux/i386/fxstat.c b/sysdeps/unix/sysv/linux/i386/fxstat.c index 2f7a8fe..646d616 100644 --- a/sysdeps/unix/sysv/linux/i386/fxstat.c +++ b/sysdeps/unix/sysv/linux/i386/fxstat.c @@ -42,10 +42,12 @@ __fxstat (int vers, int fd, struct stat *buf) { struct stat64 buf64; - result = INLINE_SYSCALL (fstat64, 2, fd, &buf64); - if (result == 0) - result = __xstat32_conv (vers, &buf64, buf); - return result; + INTERNAL_SYSCALL_DECL (err); + result = INTERNAL_SYSCALL (fstat64, err, 2, fd, &buf64); + if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (result, err))) + return INLINE_SYSCALL_ERROR_RETURN_VALUE (-result); + else + return __xstat32_conv (vers, &buf64, buf); } } diff --git a/sysdeps/unix/sysv/linux/i386/fxstatat.c b/sysdeps/unix/sysv/linux/i386/fxstatat.c index 6f3c251..4b11000 100644 --- a/sysdeps/unix/sysv/linux/i386/fxstatat.c +++ b/sysdeps/unix/sysv/linux/i386/fxstatat.c @@ -42,13 +42,10 @@ __fxstatat (int vers, int fd, const char *file, struct stat *st, int flag) struct stat64 st64; result = INTERNAL_SYSCALL (fstatat64, err, 4, fd, file, &st64, flag); - if (!__builtin_expect (INTERNAL_SYSCALL_ERROR_P (result, err), 1)) - return __xstat32_conv (vers, &st64, st); + if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (result, err))) + return INLINE_SYSCALL_ERROR_RETURN_VALUE (-result); else - { - __set_errno (INTERNAL_SYSCALL_ERRNO (result, err)); - return -1; - } + return __xstat32_conv (vers, &st64, st); } libc_hidden_def (__fxstatat) #ifdef XSTAT_IS_XSTAT64 diff --git a/sysdeps/unix/sysv/linux/i386/lockf64.c b/sysdeps/unix/sysv/linux/i386/lockf64.c index 61fcf22..601af78 100644 --- a/sysdeps/unix/sysv/linux/i386/lockf64.c +++ b/sysdeps/unix/sysv/linux/i386/lockf64.c @@ -29,6 +29,7 @@ lockf64 (int fd, int cmd, off64_t len64) { struct flock64 fl64; int cmd64; + int result; memset ((char *) &fl64, '\0', sizeof (fl64)); fl64.l_whence = SEEK_CUR; @@ -41,12 +42,13 @@ lockf64 (int fd, int cmd, off64_t len64) /* Test the lock: return 0 if FD is unlocked or locked by this process; return -1, set errno to EACCES, if another process holds the lock. */ fl64.l_type = F_RDLCK; - if (INLINE_SYSCALL (fcntl64, 3, fd, F_GETLK64, &fl64) < 0) - return -1; + INTERNAL_SYSCALL_DECL (err); + result = INTERNAL_SYSCALL (fcntl64, err, 3, fd, F_GETLK64, &fl64); + if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (result, err))) + return INLINE_SYSCALL_ERROR_RETURN_VALUE (-result); if (fl64.l_type == F_UNLCK || fl64.l_pid == __getpid ()) return 0; - __set_errno (EACCES); - return -1; + return INLINE_SYSCALL_ERROR_RETURN_VALUE (EACCES); case F_ULOCK: fl64.l_type = F_UNLCK; cmd64 = F_SETLK64; @@ -61,8 +63,7 @@ lockf64 (int fd, int cmd, off64_t len64) break; default: - __set_errno (EINVAL); - return -1; + return INLINE_SYSCALL_ERROR_RETURN_VALUE (EINVAL); } return INLINE_SYSCALL (fcntl64, 3, fd, cmd64, &fl64); } diff --git a/sysdeps/unix/sysv/linux/i386/lxstat.c b/sysdeps/unix/sysv/linux/i386/lxstat.c index 0891127..bd62774 100644 --- a/sysdeps/unix/sysv/linux/i386/lxstat.c +++ b/sysdeps/unix/sysv/linux/i386/lxstat.c @@ -43,10 +43,12 @@ __lxstat (int vers, const char *name, struct stat *buf) { struct stat64 buf64; - result = INLINE_SYSCALL (lstat64, 2, name, &buf64); - if (result == 0) - result = __xstat32_conv (vers, &buf64, buf); - return result; + INTERNAL_SYSCALL_DECL (err); + result = INTERNAL_SYSCALL (lstat64, err, 2, name, &buf64); + if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (result, err))) + return INLINE_SYSCALL_ERROR_RETURN_VALUE (-result); + else + return __xstat32_conv (vers, &buf64, buf); } } diff --git a/sysdeps/unix/sysv/linux/i386/setegid.c b/sysdeps/unix/sysv/linux/i386/setegid.c index 8c39784..c74e982 100644 --- a/sysdeps/unix/sysv/linux/i386/setegid.c +++ b/sysdeps/unix/sysv/linux/i386/setegid.c @@ -27,10 +27,7 @@ setegid (gid) int result; if (gid == (gid_t) ~0) - { - __set_errno (EINVAL); - return -1; - } + return INLINE_SYSCALL_ERROR_RETURN_VALUE (EINVAL); result = INLINE_SETXID_SYSCALL (setresgid32, 3, -1, gid, -1); diff --git a/sysdeps/unix/sysv/linux/i386/seteuid.c b/sysdeps/unix/sysv/linux/i386/seteuid.c index d6a7a27..91db7fb 100644 --- a/sysdeps/unix/sysv/linux/i386/seteuid.c +++ b/sysdeps/unix/sysv/linux/i386/seteuid.c @@ -26,10 +26,7 @@ seteuid (uid_t uid) int result; if (uid == (uid_t) ~0) - { - __set_errno (EINVAL); - return -1; - } + return INLINE_SYSCALL_ERROR_RETURN_VALUE (EINVAL); result = INLINE_SETXID_SYSCALL (setresuid32, 3, -1, uid, -1); diff --git a/sysdeps/unix/sysv/linux/i386/sigaction.c b/sysdeps/unix/sysv/linux/i386/sigaction.c index b20a9b9..6cada82 100644 --- a/sysdeps/unix/sysv/linux/i386/sigaction.c +++ b/sysdeps/unix/sysv/linux/i386/sigaction.c @@ -69,11 +69,13 @@ __libc_sigaction (int sig, const struct sigaction *act, struct sigaction *oact) /* XXX The size argument hopefully will have to be changed to the real size of the user-level sigset_t. */ - result = INLINE_SYSCALL (rt_sigaction, 4, - sig, act ? &kact : NULL, - oact ? &koact : NULL, _NSIG / 8); - - if (oact && result >= 0) + INTERNAL_SYSCALL_DECL (err); + result = INTERNAL_SYSCALL (rt_sigaction, err, 4, + sig, act ? &kact : NULL, + oact ? &koact : NULL, _NSIG / 8); + if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (result, err))) + return INLINE_SYSCALL_ERROR_RETURN_VALUE (-result); + else if (oact && result >= 0) { oact->sa_handler = koact.k_sa_handler; memcpy (&oact->sa_mask, &koact.sa_mask, sizeof (sigset_t)); diff --git a/sysdeps/unix/sysv/linux/i386/xstat.c b/sysdeps/unix/sysv/linux/i386/xstat.c index 2424434..c588f4a 100644 --- a/sysdeps/unix/sysv/linux/i386/xstat.c +++ b/sysdeps/unix/sysv/linux/i386/xstat.c @@ -43,10 +43,12 @@ __xstat (int vers, const char *name, struct stat *buf) { struct stat64 buf64; - result = INLINE_SYSCALL (stat64, 2, name, &buf64); - if (result == 0) - result = __xstat32_conv (vers, &buf64, buf); - return result; + INTERNAL_SYSCALL_DECL (err); + result = INTERNAL_SYSCALL (stat64, err, 2, name, &buf64); + if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (result, err))) + return INLINE_SYSCALL_ERROR_RETURN_VALUE (-result); + else + return __xstat32_conv (vers, &buf64, buf); } } hidden_def (__xstat) -- 2.4.3