Paul Eggert wrote:
> On this platform, <sys/time.h> declares utimens and lutimens and the
> C library defines them
But the functions defined in NetBSD's libc don't have the same
semantics as the one in Gnulib:
- They don't validate their timespec[2] argument.
- They don't bump the ctime when making a change.
The CI reported this test failure:
FAIL: test-utimens
==================
../../gltests/test-utimens.h:80: assertion 'func (BASE "file", ts) == -1' failed
Stack trace:
0x402e3c test_utimens
../../gltests/test-utimens.h:80
0x41e7d2 main
../../gltests/test-utimens.c:71
../../gltests/test-utimens.h:90: assertion 'func (BASE "file", ts) == -1' failed
Stack trace:
0x402f04 test_utimens
../../gltests/test-utimens.h:90
0x41e7d2 main
../../gltests/test-utimens.c:71
../../gltests/test-utimens.h:103: assertion 'st1.st_atime == st2.st_atime'
failed
Stack trace:
0x403043 test_utimens
../../gltests/test-utimens.h:103
0x41e7d2 main
../../gltests/test-utimens.c:71
../../gltests/test-lutimens.h:132: assertion 'func (BASE "link", ts) == -1'
failed
Stack trace:
0x404fbc test_lutimens
../../gltests/test-lutimens.h:132
0x41e840 main
../../gltests/test-utimens.c:75
../../gltests/test-lutimens.h:142: assertion 'func (BASE "link", ts) == -1'
failed
Stack trace:
0x404f3e test_lutimens
../../gltests/test-lutimens.h:142
0x41e840 main
../../gltests/test-utimens.c:75
../../gltests/test-lutimens.h:148: assertion 'st1.st_atime == st2.st_atime'
failed
Stack trace:
0x405b4d test_lutimens
../../gltests/test-lutimens.h:148
0x41e840 main
../../gltests/test-utimens.c:75
This patch fixes it.
2024-09-23 Bruno Haible <[email protected]>
utimens: Fix test failure on NetBSD 10 (regression 2024-09-16).
* lib/utimens.h (utimens): Declare as an override if utimens exists in
libc.
(lutimens): Declare as an override if lutimens exists in libc.
* lib/utimens.c (is_valid_timespec, is_valid_timespecs): New functions,
extracted from validate_timespec.
(validate_timespec): Call is_valid_timespecs.
(utimens, lutimens): On NetBSD, validate the argument before calling
NetBSD's libc function.
* tests/test-utimens-common.h (check_ctime): Set to -1 on NetBSD.
diff --git a/lib/utimens.c b/lib/utimens.c
index cd86a44ea7..3c81b5c349 100644
--- a/lib/utimens.c
+++ b/lib/utimens.c
@@ -78,6 +78,21 @@ static int utimensat_works_really;
static int lutimensat_works_really;
#endif /* HAVE_UTIMENSAT || HAVE_FUTIMENS */
+static bool
+is_valid_timespec (struct timespec const *timespec)
+{
+ return (timespec->tv_nsec == UTIME_NOW
+ || timespec->tv_nsec == UTIME_OMIT
+ || (0 <= timespec->tv_nsec && timespec->tv_nsec < TIMESPEC_HZ));
+}
+
+static bool
+is_valid_timespecs (struct timespec const timespec[2])
+{
+ return (is_valid_timespec (×pec[0])
+ && is_valid_timespec (×pec[1]));
+}
+
/* Validate the requested timestamps. Return 0 if the resulting
timespec can be used for utimensat (after possibly modifying it to
work around bugs in utimensat). Return a positive value if the
@@ -90,14 +105,7 @@ validate_timespec (struct timespec timespec[2])
{
int result = 0;
int utime_omit_count = 0;
- if ((timespec[0].tv_nsec != UTIME_NOW
- && timespec[0].tv_nsec != UTIME_OMIT
- && ! (0 <= timespec[0].tv_nsec
- && timespec[0].tv_nsec < TIMESPEC_HZ))
- || (timespec[1].tv_nsec != UTIME_NOW
- && timespec[1].tv_nsec != UTIME_OMIT
- && ! (0 <= timespec[1].tv_nsec
- && timespec[1].tv_nsec < TIMESPEC_HZ)))
+ if (!is_valid_timespecs (timespec))
{
errno = EINVAL;
return -1;
@@ -516,24 +524,44 @@ fdutimens (int fd, char const *file, struct timespec
const timespec[2])
}
}
-#if !HAVE_UTIMENS
/* Set the access and modification timestamps of FILE to be
TIMESPEC[0] and TIMESPEC[1], respectively. */
int
utimens (char const *file, struct timespec const timespec[2])
+#undef utimens
{
+#if HAVE_UTIMENS
+ /* NetBSD's native utimens() does not fulfil the Gnulib expectations:
+ At least in NetBSD 10.0, it does not validate the timespec argument. */
+ if (timespec != NULL && !is_valid_timespecs (timespec))
+ {
+ errno = EINVAL;
+ return -1;
+ }
+ return utimens (file, timespec);
+#else
return fdutimens (-1, file, timespec);
-}
#endif
+}
-#if !HAVE_LUTIMENS
/* Set the access and modification timestamps of FILE to be
TIMESPEC[0] and TIMESPEC[1], respectively, without dereferencing
symlinks. Fail with ENOSYS if the platform does not support
changing symlink timestamps, but FILE was a symlink. */
int
lutimens (char const *file, struct timespec const timespec[2])
+#undef lutimens
{
+#if HAVE_LUTIMENS
+ /* NetBSD's native lutimens() does not fulfil the Gnulib expectations:
+ At least in NetBSD 10.0, it does not validate the timespec argument. */
+ if (timespec != NULL && !is_valid_timespecs (timespec))
+ {
+ errno = EINVAL;
+ return -1;
+ }
+ return lutimens (file, timespec);
+#else
struct timespec adjusted_timespec[2];
struct timespec *ts = timespec ? adjusted_timespec : NULL;
int adjustment_needed = 0;
@@ -553,11 +581,11 @@ lutimens (char const *file, struct timespec const
timespec[2])
fdutimens' worry about buggy NFS clients. But we do have to
worry about bogus return values. */
-#if HAVE_UTIMENSAT
+# if HAVE_UTIMENSAT
if (0 <= lutimensat_works_really)
{
int result;
-# if defined __linux__ || defined __sun || defined __NetBSD__
+# if defined __linux__ || defined __sun || defined __NetBSD__
/* As recently as Linux kernel 2.6.32 (Dec 2009), several file
systems (xfs, ntfs-3g) have bugs with a single UTIME_OMIT,
but work if both times are either explicitly specified or
@@ -582,9 +610,9 @@ lutimens (char const *file, struct timespec const
timespec[2])
/* Note that st is good, in case utimensat gives ENOSYS. */
adjustment_needed++;
}
-# endif
+# endif
result = utimensat (AT_FDCWD, file, ts, AT_SYMLINK_NOFOLLOW);
-# ifdef __linux__
+# ifdef __linux__
/* Work around a kernel bug:
https://bugzilla.redhat.com/show_bug.cgi?id=442352
https://bugzilla.redhat.com/show_bug.cgi?id=449910
@@ -594,7 +622,7 @@ lutimens (char const *file, struct timespec const
timespec[2])
are no longer in common use. */
if (0 < result)
errno = ENOSYS;
-# endif
+# endif
if (result == 0 || errno != ENOSYS)
{
utimensat_works_really = 1;
@@ -603,7 +631,7 @@ lutimens (char const *file, struct timespec const
timespec[2])
}
}
lutimensat_works_really = -1;
-#endif /* HAVE_UTIMENSAT */
+# endif /* HAVE_UTIMENSAT */
/* The platform lacks an interface to set file timestamps with
nanosecond resolution, so do the best we can, discarding any
@@ -619,7 +647,7 @@ lutimens (char const *file, struct timespec const
timespec[2])
/* On Linux, lutimes is a thin wrapper around utimensat, so there is
no point trying lutimes if utimensat failed with ENOSYS. */
-#if HAVE_LUTIMES && !HAVE_UTIMENSAT
+# if HAVE_LUTIMES && !HAVE_UTIMENSAT
{
struct timeval timeval[2];
struct timeval *t;
@@ -639,7 +667,7 @@ lutimens (char const *file, struct timespec const
timespec[2])
if (result == 0 || errno != ENOSYS)
return result;
}
-#endif /* HAVE_LUTIMES && !HAVE_UTIMENSAT */
+# endif /* HAVE_LUTIMES && !HAVE_UTIMENSAT */
/* Out of luck for symlinks, but we still handle regular files. */
if (!(adjustment_needed || REPLACE_FUNC_STAT_FILE) && lstat (file, &st))
@@ -648,5 +676,5 @@ lutimens (char const *file, struct timespec const
timespec[2])
return fdutimens (-1, file, ts);
errno = ENOSYS;
return -1;
-}
#endif
+}
diff --git a/lib/utimens.h b/lib/utimens.h
index e85477b849..762c3f9a85 100644
--- a/lib/utimens.h
+++ b/lib/utimens.h
@@ -33,12 +33,16 @@ extern "C" {
#endif
int fdutimens (int, char const *, struct timespec const [2]);
-#if !HAVE_UTIMENS
+
+#if HAVE_UTIMENS
+# define utimens rpl_utimens
+#endif
int utimens (char const *, struct timespec const [2]);
+
+#if HAVE_LUTIMENS
+# define lutimens rpl_lutimens
#endif
-#if !HAVE_LUTIMENS
int lutimens (char const *, struct timespec const [2]);
-#endif
#ifdef __cplusplus
}
diff --git a/tests/test-utimens-common.h b/tests/test-utimens-common.h
index a59e65df8c..f564323630 100644
--- a/tests/test-utimens-common.h
+++ b/tests/test-utimens-common.h
@@ -55,7 +55,7 @@ enum {
properly tracked change time. See
<https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/stat-functions>.
*/
# define check_ctime 0
-# elif defined __APPLE__ && defined __MACH__
+# elif (defined __APPLE__ && defined __MACH__) || defined __NetBSD__
/* On macOS, the ctime is not updated when only the st_atime changes. */
# define check_ctime -1
# else
