This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 4/6] Use INTERNAL_SYSCALL and INLINE_SYSCALL_ERROR_RETURN


On Tue, Oct 13, 2015 at 09:37:48AM +0200, Andreas Schwab wrote:
> "H.J. Lu" <hongjiu.lu@intel.com> 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.

Thanks.

H.J.
---
>From da2730ae488af1223269ebdea9b9e779bf424c3f Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Mon, 14 Sep 2015 07:00:59 -0700
Subject: [PATCH 4/6] Use INTERNAL_SYSCALL and INLINE_SYSCALL_ERROR_RETURN

This patch uses INTERNAL_SYSCALL and INLINE_SYSCALL_ERROR_RETURN 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.
	* 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.
	* 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..2d2daa5 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 (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..dcf2ed3 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 (-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..f26265b 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 (-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..6d1d459 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 (-result);
       if (fl64.l_type == F_UNLCK || fl64.l_pid == __getpid ())
         return 0;
-      __set_errno (EACCES);
-      return -1;
+      return INLINE_SYSCALL_ERROR_RETURN (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 (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..cfe4c90 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 (-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..2525ed2 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 (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..6f31214 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 (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..05c4c17 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 (-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..792dc93 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 (-result);
+    else
+      return __xstat32_conv (vers, &buf64, buf);
   }
 }
 hidden_def (__xstat)
-- 
2.4.3


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]