| Message ID | 54AC160F.6030403@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
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.
diff --git a/sysdeps/unix/sysv/linux/futimens.c b/sysdeps/unix/sysv/linux/futimens.c index c7d03a8..1dda738f 100644 --- a/sysdeps/unix/sysv/linux/futimens.c +++ b/sysdeps/unix/sysv/linux/futimens.c @@ -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; diff --git a/sysdeps/unix/sysv/linux/futimesat.c b/sysdeps/unix/sysv/linux/futimesat.c index ac96e2a..e2cba1a 100644 --- a/sysdeps/unix/sysv/linux/futimesat.c +++ b/sysdeps/unix/sysv/linux/futimesat.c @@ -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]); } diff --git a/sysdeps/unix/sysv/linux/utimensat.c b/sysdeps/unix/sysv/linux/utimensat.c index 5a5dec7..24fd13a 100644 --- a/sysdeps/unix/sysv/linux/utimensat.c +++ b/sysdeps/unix/sysv/linux/utimensat.c @@ -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; diff --git a/sysdeps/unix/sysv/linux/utimes.c b/sysdeps/unix/sysv/linux/utimes.c index 5423ff2..8864e48 100644 --- a/sysdeps/unix/sysv/linux/utimes.c +++ b/sysdeps/unix/sysv/linux/utimes.c @@ -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)