powerpc: Fix compiler warning on some syscalls
Commit Message
On 06-01-2015 13:55, Paul Eggert wrote:
> On 01/06/2015 07:10 AM, Adhemerval Zanella wrote:
>>> Does &tvp[0] work instead?
>>> >
>>> >Andreas.
>>> >
>> It does and I think using would not require an explicit comment about it
>> usage. Would it be a preferable solution?
>>
>
> Absolutely. Casts are too powerful and can get one into trouble too easily.
>
Fair enough, is it ok now for push?
--
* sysdeps/unix/sysv/linux/futimens.c (futimens): Use address of first
timespec struct member in syscall macro.
* sysdeps/unix/sysv/linux/utimensat.c (utimensat): Likewise.
* sysdeps/unix/sysv/linux/futimesat.c (futimesat): Use address of
first timeval struct member in syscall macro.
* sysdeps/unix/sysv/linux/utimes.c (__utimeS): Likewise.
--
Comments
Using &foo[0] is fine, but I think it does need a comment in each place.
It doesn't have to be a full explanation, just something short like:
/* Avoid implicit array coercion in syscall macros. */
Otherwise someone will come along at some point and say, "&foo[0] is
equivalent to foo in C, so I'll remove the superfluous operators."
A more full explanation can go with the definition of INLINE_SYSCALL et al.
It wouldn't hurt for at least some of those implementations to add a
__builtin_classify_type check so that any array_type_class arguments to
syscall macros (at least in machine-independent code) barf at compile time
in a way that leads to seeing a comment that explains the situation.
Thanks,
Roland
On 07-01-2015 21:31, Roland McGrath wrote:
> Using &foo[0] is fine, but I think it does need a comment in each place.
> It doesn't have to be a full explanation, just something short like:
> /* Avoid implicit array coercion in syscall macros. */
> Otherwise someone will come along at some point and say, "&foo[0] is
> equivalent to foo in C, so I'll remove the superfluous operators."
> A more full explanation can go with the definition of INLINE_SYSCALL et al.
>
> It wouldn't hurt for at least some of those implementations to add a
> __builtin_classify_type check so that any array_type_class arguments to
> syscall macros (at least in machine-independent code) barf at compile time
> in a way that leads to seeing a comment that explains the situation.
>
>
> Thanks,
> Roland
>
I have added your comment suggestion and pushed upstream as
dd6e8af6ba1b8a95a7f1dc7422e5ea4ccc7fbd93.
@@ -37,7 +37,7 @@ futimens (int fd, const struct timespec tsp[2])
__set_errno (EBADF);
return -1;
}
- return INLINE_SYSCALL (utimensat, 4, fd, NULL, tsp, 0);
+ return INLINE_SYSCALL (utimensat, 4, fd, NULL, &tsp[0], 0);
#else
__set_errno (ENOSYS);
return -1;
@@ -28,13 +28,10 @@
/* 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);
+ return INLINE_SYSCALL (futimesat, 3, fd, file, &tvp[0]);
}
@@ -35,7 +35,7 @@ utimensat (int fd, const char *file, const struct timespec tsp[2],
return -1;
}
#ifdef __NR_utimensat
- return INLINE_SYSCALL (utimensat, 4, fd, file, tsp, flags);
+ return INLINE_SYSCALL (utimensat, 4, fd, file, &tsp[0], flags);
#else
__set_errno (ENOSYS);
return -1;
@@ -29,7 +29,7 @@
int
__utimes (const char *file, const struct timeval tvp[2])
{
- return INLINE_SYSCALL (utimes, 2, file, tvp);
+ return INLINE_SYSCALL (utimes, 2, file, &tvp[0]);
}
weak_alias (__utimes, utimes)