This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] powerpc: Fix compiler warning on some syscalls
- From: Marek Polacek <polacek at redhat dot com>
- To: Adhemerval Zanella <azanella at linux dot vnet dot ibm dot com>
- Cc: "GNU C. Library" <libc-alpha at sourceware dot org>
- Date: Tue, 6 Jan 2015 14:50:16 +0100
- Subject: Re: [PATCH] powerpc: Fix compiler warning on some syscalls
- Authentication-results: sourceware.org; auth=none
- References: <54ABDECE dot 3030306 at linux dot vnet dot ibm dot com> <20150106131810 dot GB16430 at redhat dot com> <54ABE61C dot 8090108 at linux dot vnet dot ibm dot com>
On Tue, Jan 06, 2015 at 11:41:48AM -0200, Adhemerval Zanella wrote:
> On 06-01-2015 11:18, Marek Polacek wrote:
> > On Tue, Jan 06, 2015 at 11:10:38AM -0200, Adhemerval Zanella wrote:
> >> GCC 5.0 emits an warning when using sizeof on array function parameters
> >> and powerpc internal syscall macros add an check for such cases. More
> >> specifically, on powerpc64 and powerpc32 sysdep.h:
> >>
> >> if (__builtin_classify_type (__arg3) != 5 && sizeof (__arg3) > 8) \
> >> __illegally_sized_syscall_arg3 (); \
> >>
> >> And for sysdeps/unix/sysv/linux/utimensat.c build GCC emits:
> >>
> >> error: âsizeofâ on array function parameter âtspâ will return size of
> >> âconst struct timespec *â
> >>
> >> This patch adds explicit casts to struct pointers to avoid the warnings.
> >>
> >> Checked on powerpc64 and powerpc32. Ok to push?
> >>
> >> PS: it turned out my earlier checks were incorrect and I didn't see the
> >> issue when I updated my GCC because I used --disable-werror.
> >>
> >> --
> >>
> >> * sysdeps/unix/sysv/linux/futimens.c (futimens): Cast timespec struct
> >> argument to pointer.
> >> * sysdeps/unix/sysv/linux/utimensat.c (utimensat): Likewise.
> >> * sysdeps/unix/sysv/linux/futimesat.c (futimesat): Cast timeval struct
> >> argument to pointer.
> >> * sysdeps/unix/sysv/linux/utimes.c (__utimeS): Likewise.
> >>
> >> --
> >>
> >> diff --git a/sysdeps/unix/sysv/linux/futimens.c b/sysdeps/unix/sysv/linux/futimens.c
> >> index c7d03a8..c0e7219 100644
> >> --- a/sysdeps/unix/sysv/linux/futimens.c
> >> +++ b/sysdeps/unix/sysv/linux/futimens.c
> >> @@ -37,7 +37,11 @@ futimens (int fd, const struct timespec tsp[2])
> >> __set_errno (EBADF);
> >> return -1;
> >> }
> >> - return INLINE_SYSCALL (utimensat, 4, fd, NULL, tsp, 0);
> >> + /* Some archs (powerpc) add arguments type and size check using sizeof
> >> + and without a cast the compiler might emit an warning about using
> >> + sizeof on a struct (where the builtin returns the pointer size). */
> >> + return INLINE_SYSCALL (utimensat, 4, fd, NULL, (const struct timespec*)tsp,
> >> + 0);
> >> #else
> >> __set_errno (ENOSYS);
> >> return -1;
> >> diff --git a/sysdeps/unix/sysv/linux/futimesat.c b/sysdeps/unix/sysv/linux/futimesat.c
> >> index ac96e2a..f7d5645 100644
> >> --- a/sysdeps/unix/sysv/linux/futimesat.c
> >> +++ b/sysdeps/unix/sysv/linux/futimesat.c
> >> @@ -28,13 +28,13 @@
> >> /* Change the access time of FILE relative to FD to TVP[0] and
> >> the modification time of FILE to TVP[1]. */
> >> int
> >> -futimesat (fd, file, tvp)
> >> - int fd;
> >> - const char *file;
> >> - const struct timeval tvp[2];
> >> +futimesat (int fd, const char *file, const struct timeval tvp[2])
> >> {
> >> if (file == NULL)
> >> return __futimes (fd, tvp);
> >>
> >> - return INLINE_SYSCALL (futimesat, 3, fd, file, tvp);
> >> + /* Some archs (powerpc) add arguments type and size check using sizeof
> >> + and without a cast the compiler might emit an warning about using
> >> + sizeof on a struct (where the builtin returns the pointer size). */
> >> + return INLINE_SYSCALL (futimesat, 3, fd, file, (const struct timeval*)tvp);
> > I think it'd be better to just change the function parameter to const
> > struct timeval *tvp; the warning is meant to detect a case when a sizeof
> > is applied to a function parameter declared as an array.
> >
> > Marek
> >
> I personally don't have a preference, this suggestion was given by Joseph in a
> previous threads requesting for comments. I only see that pushing your suggestion
> will require more changes (the headers and manual).
Allright, in that case please watch for formatting,
(const struct timeval*)tvp
should be
(const struct timeval *) tvp
Marek