[2/8] Consolidate Linux open implementation

Message ID 1494257212-524-2-git-send-email-adhemerval.zanella@linaro.org
State New
Headers show
Series
  • [1/8] Consolidate Linux close syscall generation
Related show

Commit Message

Adhemerval Zanella May 8, 2017, 3:26 p.m.
This patch consolidates the open Linux syscall implementation on
sysdeps/unix/sysv/linux/open{64}.c.  The changes are:

  1. Remove open{64} from auto-generation syscalls.list.
  2. Add a new open{64}.c implementation.  For architectures that
     define __OFF_T_MATCHES_OFF64_T the default open64 will create
     alias to required open symbols.
  3. Use __NR_open where possible, otherwise use __NR_openat.

Checked on i686-linux-gnu, x86_64-linux-gnu, x86_64-linux-gnux32,
arch64-linux-gnu, arm-linux-gnueabihf, and powerpc64le-linux-gnu.

	* sysdeps/unix/sysv/linux/generic/open.c: Remove file.
	* sysdeps/unix/sysv/linux/generic/open64.c: Likewise.
	* sysdeps/unix/sysv/linux/wordsize-64/open64.c: Likewise.
	* sysdeps/unix/sysv/linux/open.c: New file.
	* sysdeps/unix/sysv/linux/open64.c (__libc_open64): Use O_LARGEFILE
	only for __OFF_T_MATCHES_OFF64_T and add alias to open if the case.
	* sysdeps/unix/sysv/linux/wordsize-64/syscalls.list: Remove open
	from auto-generated list.
---
 ChangeLog                                         |  9 +++++
 sysdeps/unix/sysv/linux/generic/open64.c          | 44 -----------------------
 sysdeps/unix/sysv/linux/{generic => }/open.c      | 29 ++++++---------
 sysdeps/unix/sysv/linux/open64.c                  | 28 ++++++++++++---
 sysdeps/unix/sysv/linux/wordsize-64/open64.c      |  1 -
 sysdeps/unix/sysv/linux/wordsize-64/syscalls.list |  1 -
 6 files changed, 44 insertions(+), 68 deletions(-)
 delete mode 100644 sysdeps/unix/sysv/linux/generic/open64.c
 rename sysdeps/unix/sysv/linux/{generic => }/open.c (78%)
 delete mode 100644 sysdeps/unix/sysv/linux/wordsize-64/open64.c

-- 
2.7.4

Comments

Andreas Schwab May 11, 2017, 3:06 p.m. | #1
On Mai 08 2017, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:

> This patch consolidates the open Linux syscall implementation on

> sysdeps/unix/sysv/linux/open{64}.c.  The changes are:

>

>   1. Remove open{64} from auto-generation syscalls.list.

>   2. Add a new open{64}.c implementation.  For architectures that

>      define __OFF_T_MATCHES_OFF64_T the default open64 will create

>      alias to required open symbols.

>   3. Use __NR_open where possible, otherwise use __NR_openat.


Perhaps we should use __NR_openat unconditionally?  (Even 2.6.32 already
had it.)

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
Adhemerval Zanella May 11, 2017, 3:19 p.m. | #2
On 11/05/2017 12:06, Andreas Schwab wrote:
> On Mai 08 2017, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:

> 

>> This patch consolidates the open Linux syscall implementation on

>> sysdeps/unix/sysv/linux/open{64}.c.  The changes are:

>>

>>   1. Remove open{64} from auto-generation syscalls.list.

>>   2. Add a new open{64}.c implementation.  For architectures that

>>      define __OFF_T_MATCHES_OFF64_T the default open64 will create

>>      alias to required open symbols.

>>   3. Use __NR_open where possible, otherwise use __NR_openat.

> 

> Perhaps we should use __NR_openat unconditionally?  (Even 2.6.32 already

> had it.)

> 

> Andreas.

> 


I would prefer also, although using __NR_open is a marginal optimization 
(which I do not think it would mater).  I will change it.
Tulio Magno Quites Machado Filho May 15, 2017, 12:47 p.m. | #3
Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:

> This patch consolidates the open Linux syscall implementation on

> sysdeps/unix/sysv/linux/open{64}.c.  The changes are:

>

>   1. Remove open{64} from auto-generation syscalls.list.

>   2. Add a new open{64}.c implementation.  For architectures that

>      define __OFF_T_MATCHES_OFF64_T the default open64 will create

>      alias to required open symbols.

>   3. Use __NR_open where possible, otherwise use __NR_openat.

>

> Checked on i686-linux-gnu, x86_64-linux-gnu, x86_64-linux-gnux32,

> arch64-linux-gnu, arm-linux-gnueabihf, and powerpc64le-linux-gnu.

>

> 	* sysdeps/unix/sysv/linux/generic/open.c: Remove file.

> 	* sysdeps/unix/sysv/linux/generic/open64.c: Likewise.

> 	* sysdeps/unix/sysv/linux/wordsize-64/open64.c: Likewise.

> 	* sysdeps/unix/sysv/linux/open.c: New file.

> 	* sysdeps/unix/sysv/linux/open64.c (__libc_open64): Use O_LARGEFILE

> 	only for __OFF_T_MATCHES_OFF64_T and add alias to open if the case.

> 	* sysdeps/unix/sysv/linux/wordsize-64/syscalls.list: Remove open

> 	from auto-generated list.


For the record: there was a build error in our POWER8 BuildSlave right after
this patch was merged [1].

I was not able to reproduce this error on my servers and I suspect it might
be related to issues in our BuildBot setup which may still be leaving
old files before starting a new build.

This patch is not related to the failures on build 464 on the same builder.

[1] http://144.217.14.79/builders/glibc-power8-linux/builds/463

-- 
Tulio Magno
Adhemerval Zanella May 15, 2017, 1 p.m. | #4
On 15/05/2017 09:47, Tulio Magno Quites Machado Filho wrote:
> Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:

> 

>> This patch consolidates the open Linux syscall implementation on

>> sysdeps/unix/sysv/linux/open{64}.c.  The changes are:

>>

>>   1. Remove open{64} from auto-generation syscalls.list.

>>   2. Add a new open{64}.c implementation.  For architectures that

>>      define __OFF_T_MATCHES_OFF64_T the default open64 will create

>>      alias to required open symbols.

>>   3. Use __NR_open where possible, otherwise use __NR_openat.

>>

>> Checked on i686-linux-gnu, x86_64-linux-gnu, x86_64-linux-gnux32,

>> arch64-linux-gnu, arm-linux-gnueabihf, and powerpc64le-linux-gnu.

>>

>> 	* sysdeps/unix/sysv/linux/generic/open.c: Remove file.

>> 	* sysdeps/unix/sysv/linux/generic/open64.c: Likewise.

>> 	* sysdeps/unix/sysv/linux/wordsize-64/open64.c: Likewise.

>> 	* sysdeps/unix/sysv/linux/open.c: New file.

>> 	* sysdeps/unix/sysv/linux/open64.c (__libc_open64): Use O_LARGEFILE

>> 	only for __OFF_T_MATCHES_OFF64_T and add alias to open if the case.

>> 	* sysdeps/unix/sysv/linux/wordsize-64/syscalls.list: Remove open

>> 	from auto-generated list.

> 

> For the record: there was a build error in our POWER8 BuildSlave right after

> this patch was merged [1].

> 

> I was not able to reproduce this error on my servers and I suspect it might

> be related to issues in our BuildBot setup which may still be leaving

> old files before starting a new build.

> 

> This patch is not related to the failures on build 464 on the same builder.

> 

> [1] http://144.217.14.79/builders/glibc-power8-linux/builds/463

> 


It is unexpected because I did checked natively on powerpc64le-linux-gnu
(power8) and also on cross-compiling using build-many-glibc.py.

Patch

diff --git a/sysdeps/unix/sysv/linux/generic/open64.c b/sysdeps/unix/sysv/linux/generic/open64.c
deleted file mode 100644
index 88312a1..0000000
--- a/sysdeps/unix/sysv/linux/generic/open64.c
+++ /dev/null
@@ -1,44 +0,0 @@ 
-/* Copyright (C) 2011-2017 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-   Contributed by Chris Metcalf <cmetcalf@tilera.com>, 2011.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library.  If not, see
-   <http://www.gnu.org/licenses/>.  */
-
-#include <errno.h>
-#include <fcntl.h>
-#include <stdarg.h>
-#include <stdio.h>
-#include <sysdep-cancel.h>
-
-/* Open FILE with access OFLAG.  If O_CREAT or O_TMPFILE is in OFLAG,
-   a third argument is the file protection.  */
-int
-__libc_open64 (const char *file, int oflag, ...)
-{
-  int mode = 0;
-
-  if (__OPEN_NEEDS_MODE (oflag))
-    {
-      va_list arg;
-      va_start (arg, oflag);
-      mode = va_arg (arg, int);
-      va_end (arg);
-    }
-
-  return SYSCALL_CANCEL (openat, AT_FDCWD, file, oflag | O_LARGEFILE, mode);
-}
-weak_alias (__libc_open64, __open64)
-libc_hidden_weak (__open64)
-weak_alias (__libc_open64, open64)
diff --git a/sysdeps/unix/sysv/linux/generic/open.c b/sysdeps/unix/sysv/linux/open.c
similarity index 78%
rename from sysdeps/unix/sysv/linux/generic/open.c
rename to sysdeps/unix/sysv/linux/open.c
index 10cdbe0..9321bd4 100644
--- a/sysdeps/unix/sysv/linux/generic/open.c
+++ b/sysdeps/unix/sysv/linux/open.c
@@ -1,4 +1,4 @@ 
-/* Copyright (C) 2011-2017 Free Software Foundation, Inc.
+/* Copyright (C) 2017 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Contributed by Chris Metcalf <cmetcalf@tilera.com>, 2011.
 
@@ -16,12 +16,15 @@ 
    License along with the GNU C Library.  If not, see
    <http://www.gnu.org/licenses/>.  */
 
-#include <errno.h>
+#include <sys/types.h>
+#include <sys/stat.h>
 #include <fcntl.h>
 #include <stdarg.h>
-#include <stdio.h>
+
 #include <sysdep-cancel.h>
 
+#ifndef __OFF_T_MATCHES_OFF64_T
+
 /* Open FILE with access OFLAG.  If O_CREAT or O_TMPFILE is in OFLAG,
    a third argument is the file protection.  */
 int
@@ -37,7 +40,11 @@  __libc_open (const char *file, int oflag, ...)
       va_end (arg);
     }
 
+# ifdef __NR_open
+  return SYSCALL_CANCEL (open, file, oflag, mode);
+# else
   return SYSCALL_CANCEL (openat, AT_FDCWD, file, oflag, mode);
+# endif
 }
 libc_hidden_def (__libc_open)
 
@@ -45,18 +52,4 @@  weak_alias (__libc_open, __open)
 libc_hidden_weak (__open)
 weak_alias (__libc_open, open)
 
-int
-__open_nocancel (const char *file, int oflag, ...)
-{
-  int mode = 0;
-
-  if (__OPEN_NEEDS_MODE (oflag))
-    {
-      va_list arg;
-      va_start (arg, oflag);
-      mode = va_arg (arg, int);
-      va_end (arg);
-    }
-
-  return INLINE_SYSCALL (openat, 4, AT_FDCWD, file, oflag, mode);
-}
+#endif
diff --git a/sysdeps/unix/sysv/linux/open64.c b/sysdeps/unix/sysv/linux/open64.c
index 5e209ee..6ad8f77 100644
--- a/sysdeps/unix/sysv/linux/open64.c
+++ b/sysdeps/unix/sysv/linux/open64.c
@@ -15,10 +15,11 @@ 
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
-#include <errno.h>
+#include <sys/types.h>
+#include <sys/stat.h>
 #include <fcntl.h>
 #include <stdarg.h>
-#include <stdio.h>
+
 #include <sysdep-cancel.h>
 
 /* Open FILE with access OFLAG.  If O_CREAT or O_TMPFILE is in OFLAG,
@@ -36,8 +37,27 @@  __libc_open64 (const char *file, int oflag, ...)
       va_end (arg);
     }
 
-  return SYSCALL_CANCEL (open, file, oflag | O_LARGEFILE, mode);
+#ifdef __OFF_T_MATCHES_OFF64_T
+# define EXTRA_OPEN_FLAGS 0
+#else
+# define EXTRA_OPEN_FLAGS O_LARGEFILE
+#endif
+
+#ifdef __NR_open
+  return SYSCALL_CANCEL (open, file, oflag | EXTRA_OPEN_FLAGS, mode);
+#else
+  return SYSCALL_CANCEL (openat, AT_FDCWD, file, oflag | EXTRA_OPEN_FLAGS,
+			 mode);
+#endif
 }
-weak_alias (__libc_open64, __open64)
+
+strong_alias (__libc_open64, __open64)
 libc_hidden_weak (__open64)
 weak_alias (__libc_open64, open64)
+
+#ifdef __OFF_T_MATCHES_OFF64_T
+strong_alias (__libc_open64, __libc_open)
+strong_alias (__libc_open64, __open)
+libc_hidden_weak (__open)
+weak_alias (__libc_open64, open)
+#endif
diff --git a/sysdeps/unix/sysv/linux/wordsize-64/open64.c b/sysdeps/unix/sysv/linux/wordsize-64/open64.c
deleted file mode 100644
index 0abe30e..0000000
--- a/sysdeps/unix/sysv/linux/wordsize-64/open64.c
+++ /dev/null
@@ -1 +0,0 @@ 
-/* Defined in open syscall.  */
diff --git a/sysdeps/unix/sysv/linux/wordsize-64/syscalls.list b/sysdeps/unix/sysv/linux/wordsize-64/syscalls.list
index 0c60647..6549ed8 100644
--- a/sysdeps/unix/sysv/linux/wordsize-64/syscalls.list
+++ b/sysdeps/unix/sysv/linux/wordsize-64/syscalls.list
@@ -6,7 +6,6 @@  readahead	-	readahead	i:iii	__readahead	readahead
 sendfile	-	sendfile	i:iipi	sendfile	sendfile64
 sync_file_range	-	sync_file_range	Ci:iiii	sync_file_range
 creat		-	creat		Ci:si	creat		creat64
-open		-	open		Ci:siv	__libc_open	__open open __open64 open64
 prlimit		EXTRA	prlimit64	i:iipp	prlimit		prlimit64
 
 fanotify_mark	EXTRA	fanotify_mark	i:iiiis	fanotify_mark