[v2] Handle possible vararg promotion of mode_t in open and friends

Message ID 1400538110-7066-1-git-send-email-rv@rasmusvillemoes.dk
State Superseded
Headers

Commit Message

Rasmus Villemoes May 19, 2014, 10:21 p.m. UTC
  On some platforms, mode_t is narrower than int and hence promoted to
int when passed through as the optional argument to an open-like
function; on others, mode_t is at least as wide as int and hence
passed through as-is. To accomodate the case where mode_t might be
strictly wider than int (making va_arg(ap, int) wrong to use), use
whatever type mode_t promotes to as the second argument to
va_arg. Rich Felker suggested that this could be obtained simply as
__typeof__(+(mode_t)0) (which works since POSIX insists that mode_t is
an integral type).
---
 io/open.c                                | 8 ++++----
 io/open64.c                              | 4 ++--
 io/openat.c                              | 4 ++--
 io/openat64.c                            | 4 ++--
 nptl/sem_open.c                          | 2 +-
 sysdeps/mach/hurd/open.c                 | 2 +-
 sysdeps/mach/hurd/openat.c               | 2 +-
 sysdeps/posix/open64.c                   | 4 ++--
 sysdeps/unix/sysv/linux/generic/open.c   | 8 ++++----
 sysdeps/unix/sysv/linux/generic/open64.c | 4 ++--
 sysdeps/unix/sysv/linux/mq_open.c        | 2 +-
 sysdeps/unix/sysv/linux/open64.c         | 4 ++--
 sysdeps/unix/sysv/linux/openat.c         | 2 +-
 13 files changed, 25 insertions(+), 25 deletions(-)
  

Comments

Mike Frysinger Aug. 2, 2014, 3:53 p.m. UTC | #1
On Tue 20 May 2014 00:21:50 Rasmus Villemoes wrote:
> +      mode = va_arg (ap, __typeof__(+(mode_t)0));

seems like this should be a define in a private header rather than copy & 
pasted in every call site
-mike
  
Rasmus Villemoes Aug. 3, 2014, 12:25 p.m. UTC | #2
Mike Frysinger <vapier@gentoo.org> writes:

> On Tue 20 May 2014 00:21:50 Rasmus Villemoes wrote:
>> +      mode = va_arg (ap, __typeof__(+(mode_t)0));
>
> seems like this should be a define in a private header rather than copy & 
> pasted in every call site

Good idea. I'm not too familiar with glibc's internal organization; in
particular, I don't know which headers are private and which are
not. Could you suggest a specific header file to put this in?

Thanks,
Rasmus
  
Mike Frysinger Aug. 3, 2014, 1:27 p.m. UTC | #3
On Sun 03 Aug 2014 14:25:12 Rasmus Villemoes wrote:
> Mike Frysinger <vapier@gentoo.org> writes:
> > On Tue 20 May 2014 00:21:50 Rasmus Villemoes wrote:
> >> +      mode = va_arg (ap, __typeof__(+(mode_t)0));
> > 
> > seems like this should be a define in a private header rather than copy &
> > pasted in every call site
> 
> Good idea. I'm not too familiar with glibc's internal organization; in
> particular, I don't know which headers are private and which are
> not. Could you suggest a specific header file to put this in?

i think include/fcntl.h would be best
-mike
  

Patch

diff --git a/io/open.c b/io/open.c
index 24aa380..c40a6b7 100644
--- a/io/open.c
+++ b/io/open.c
@@ -30,7 +30,7 @@  __libc_open (file, oflag)
      const char *file;
      int oflag;
 {
-  int mode;
+  mode_t mode;
 
   if (file == NULL)
     {
@@ -41,9 +41,9 @@  __libc_open (file, oflag)
   if (oflag & O_CREAT)
     {
       va_list arg;
-      va_start(arg, oflag);
-      mode = va_arg(arg, int);
-      va_end(arg);
+      va_start (arg, oflag);
+      mode = va_arg (ap, __typeof__(+(mode_t)0));
+      va_end (arg);
     }
 
   __set_errno (ENOSYS);
diff --git a/io/open64.c b/io/open64.c
index 3f3d2e8..1e3750d 100644
--- a/io/open64.c
+++ b/io/open64.c
@@ -28,7 +28,7 @@  __libc_open64 (file, oflag)
      const char *file;
      int oflag;
 {
-  int mode;
+  mode_t mode;
 
   if (file == NULL)
     {
@@ -40,7 +40,7 @@  __libc_open64 (file, oflag)
     {
       va_list arg;
       va_start (arg, oflag);
-      mode = va_arg (arg, int);
+      mode = va_arg (ap, __typeof__(+(mode_t)0));
       va_end (arg);
     }
 
diff --git a/io/openat.c b/io/openat.c
index 2d82270..c4cdf6c 100644
--- a/io/openat.c
+++ b/io/openat.c
@@ -38,7 +38,7 @@  __openat (fd, file, oflag)
      const char *file;
      int oflag;
 {
-  int mode;
+  mode_t mode;
 
   if (file == NULL)
     {
@@ -64,7 +64,7 @@  __openat (fd, file, oflag)
     {
       va_list arg;
       va_start (arg, oflag);
-      mode = va_arg (arg, int);
+      mode = va_arg (ap, __typeof__(+(mode_t)0));
       va_end (arg);
     }
 
diff --git a/io/openat64.c b/io/openat64.c
index c0c4e19..be69e5f 100644
--- a/io/openat64.c
+++ b/io/openat64.c
@@ -31,7 +31,7 @@  __openat64 (fd, file, oflag)
      const char *file;
      int oflag;
 {
-  int mode;
+  mode_t mode;
 
   if (file == NULL)
     {
@@ -57,7 +57,7 @@  __openat64 (fd, file, oflag)
     {
       va_list arg;
       va_start (arg, oflag);
-      mode = va_arg (arg, int);
+      mode = va_arg (ap, __typeof__(+(mode_t)0));
       va_end (arg);
     }
 
diff --git a/nptl/sem_open.c b/nptl/sem_open.c
index cf91859..e729acd 100644
--- a/nptl/sem_open.c
+++ b/nptl/sem_open.c
@@ -291,7 +291,7 @@  sem_open (const char *name, int oflag, ...)
     try_create:
       va_start (ap, oflag);
 
-      mode = va_arg (ap, mode_t);
+      mode = va_arg (ap, __typeof__(+(mode_t)0));
       value = va_arg (ap, unsigned int);
 
       va_end (ap);
diff --git a/sysdeps/mach/hurd/open.c b/sysdeps/mach/hurd/open.c
index 7d9b2de..e99e685 100644
--- a/sysdeps/mach/hurd/open.c
+++ b/sysdeps/mach/hurd/open.c
@@ -34,7 +34,7 @@  __libc_open (const char *file, int oflag, ...)
     {
       va_list arg;
       va_start (arg, oflag);
-      mode = va_arg (arg, mode_t);
+      mode = va_arg (ap, __typeof__(+(mode_t)0));
       va_end (arg);
     }
   else
diff --git a/sysdeps/mach/hurd/openat.c b/sysdeps/mach/hurd/openat.c
index 318cb22..438d650 100644
--- a/sysdeps/mach/hurd/openat.c
+++ b/sysdeps/mach/hurd/openat.c
@@ -41,7 +41,7 @@  __openat (fd, file, oflag)
     {
       va_list arg;
       va_start (arg, oflag);
-      mode = va_arg (arg, mode_t);
+      mode = va_arg (ap, __typeof__(+(mode_t)0));
       va_end (arg);
     }
   else
diff --git a/sysdeps/posix/open64.c b/sysdeps/posix/open64.c
index 64d192a..6615400 100644
--- a/sysdeps/posix/open64.c
+++ b/sysdeps/posix/open64.c
@@ -24,13 +24,13 @@ 
 int
 __libc_open64 (const char *file, int oflag, ...)
 {
-  int mode = 0;
+  mode_t mode = 0;
 
   if (oflag & O_CREAT)
     {
       va_list arg;
       va_start (arg, oflag);
-      mode = va_arg (arg, int);
+      mode = va_arg (ap, __typeof__(+(mode_t)0));
       va_end (arg);
     }
 
diff --git a/sysdeps/unix/sysv/linux/generic/open.c b/sysdeps/unix/sysv/linux/generic/open.c
index 4f73fa0..7098f11 100644
--- a/sysdeps/unix/sysv/linux/generic/open.c
+++ b/sysdeps/unix/sysv/linux/generic/open.c
@@ -27,13 +27,13 @@ 
 int
 __libc_open (const char *file, int oflag, ...)
 {
-  int mode = 0;
+  mode_t mode = 0;
 
   if (oflag & O_CREAT)
     {
       va_list arg;
       va_start (arg, oflag);
-      mode = va_arg (arg, int);
+      mode = va_arg (ap, __typeof__(+(mode_t)0));
       va_end (arg);
     }
 
@@ -57,13 +57,13 @@  weak_alias (__libc_open, open)
 int
 __open_nocancel (const char *file, int oflag, ...)
 {
-  int mode = 0;
+  mode_t mode = 0;
 
   if (oflag & O_CREAT)
     {
       va_list arg;
       va_start (arg, oflag);
-      mode = va_arg (arg, int);
+      mode = va_arg (ap, __typeof__(+(mode_t)0));
       va_end (arg);
     }
 
diff --git a/sysdeps/unix/sysv/linux/generic/open64.c b/sysdeps/unix/sysv/linux/generic/open64.c
index 93d79e3..ed26c8f 100644
--- a/sysdeps/unix/sysv/linux/generic/open64.c
+++ b/sysdeps/unix/sysv/linux/generic/open64.c
@@ -27,13 +27,13 @@ 
 int
 __libc_open64 (const char *file, int oflag, ...)
 {
-  int mode = 0;
+  mode_t mode = 0;
 
   if (oflag & O_CREAT)
     {
       va_list arg;
       va_start (arg, oflag);
-      mode = va_arg (arg, int);
+      mode = va_arg (ap, __typeof__(+(mode_t)0));
       va_end (arg);
     }
 
diff --git a/sysdeps/unix/sysv/linux/mq_open.c b/sysdeps/unix/sysv/linux/mq_open.c
index 38194ac..0fb38a6 100644
--- a/sysdeps/unix/sysv/linux/mq_open.c
+++ b/sysdeps/unix/sysv/linux/mq_open.c
@@ -47,7 +47,7 @@  __mq_open (const char *name, int oflag, ...)
       va_list ap;
 
       va_start (ap, oflag);
-      mode = va_arg (ap, mode_t);
+      mode = va_arg (ap, __typeof__(+(mode_t)0));
       attr = va_arg (ap, struct mq_attr *);
       va_end (ap);
     }
diff --git a/sysdeps/unix/sysv/linux/open64.c b/sysdeps/unix/sysv/linux/open64.c
index 0d63806..648fef7 100644
--- a/sysdeps/unix/sysv/linux/open64.c
+++ b/sysdeps/unix/sysv/linux/open64.c
@@ -26,13 +26,13 @@ 
 int
 __libc_open64 (const char *file, int oflag, ...)
 {
-  int mode = 0;
+  mode_t mode = 0;
 
   if (oflag & O_CREAT)
     {
       va_list arg;
       va_start (arg, oflag);
-      mode = va_arg (arg, int);
+      mode = va_arg (ap, __typeof__(+(mode_t)0));
       va_end (arg);
     }
 
diff --git a/sysdeps/unix/sysv/linux/openat.c b/sysdeps/unix/sysv/linux/openat.c
index 2cf233b..d0f06c9 100644
--- a/sysdeps/unix/sysv/linux/openat.c
+++ b/sysdeps/unix/sysv/linux/openat.c
@@ -161,7 +161,7 @@  __OPENAT (fd, file, oflag)
     {
       va_list arg;
       va_start (arg, oflag);
-      mode = va_arg (arg, mode_t);
+      mode = va_arg (ap, __typeof__(+(mode_t)0));
       va_end (arg);
     }