Re: utimens: new shadowing warning

[Eric Blake]

Eric Blake <ebb9 <at> byu.net> writes:

> I just spotted a larger logic problem - on Linux kernels between 2.6.19 and 
> 2.6.22 (when utimensat existed, but rejected AT_SYMLINK_NOFOLLOW)(, we are 
now 
> calling lstat twice when only once is necessary.  Therefore, I think the best 
> course of action is to reuse a single stat buffer, and guarantees at most one 
> lstat per lutimens.  I'll work up the patch.


From: Eric Blake <address@hidden>
Date: Thu, 31 Dec 2009 12:28:35 -0700
Subject: [PATCH] utimens: avoid shadowing warning

lutimens declared struct stat st in two scopes.  Worse, on Linux
kernels between 2.6.18 and 2.6.22 (when utimensat existed, but
rejected AT_SYMLINK_NOFOLLOW) or before 2.6.18 (if the glibc
headers have utimensat, but the kernel does not), it would
result in redundant [fl]stat calls.

* lib/utimens.c (fdutimens, lutimens): Consolidate separate stat
buffers into one, to avoid shadowing, as well as avoiding a
redundant stat.
Reported by Jim Meyering.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog     |    8 ++++++++
 lib/utimens.c |   13 ++++++++-----
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 2cd2b46..7f2dbec 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,13 @@
 2009-12-31  Eric Blake  <address@hidden>

+       utimens: avoid shadowing warning
+       * lib/utimens.c (fdutimens, lutimens): Consolidate separate stat
+       buffers into one, to avoid shadowing, as well as avoiding a
+       redundant stat.
+       Reported by Jim Meyering.
+
+2009-12-31  Eric Blake  <address@hidden>
+
        stdio: warn on suspicious uses
        * modules/stdio (Depends-on): Add warn-on-use.
        (Makefile.am): Provide new substitutions.
diff --git a/lib/utimens.c b/lib/utimens.c
index cc0d1e8..b3a9a4a 100644
--- a/lib/utimens.c
+++ b/lib/utimens.c
@@ -169,6 +169,7 @@ fdutimens (char const *file, int fd, struct timespec const 
timespec[2])
   struct timespec adjusted_timespec[2];
   struct timespec *ts = timespec ? adjusted_timespec : NULL;
   int adjustment_needed = 0;
+  struct stat st;

   if (ts)
     {
@@ -220,7 +221,6 @@ fdutimens (char const *file, int fd, struct timespec const 
timespec[2])
     {
       int result;
 # if __linux__
-      struct stat st;
       /* 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
@@ -237,6 +237,8 @@ fdutimens (char const *file, int fd, struct timespec const 
timespec[2])
             ts[0] = get_stat_atime (&st);
           else if (ts[1].tv_nsec == UTIME_OMIT)
             ts[1] = get_stat_mtime (&st);
+          /* Note that st is good, in case utimensat gives ENOSYS.  */
+          adjustment_needed++;
         }
 # endif /* __linux__ */
 # if HAVE_UTIMENSAT
@@ -287,8 +289,8 @@ fdutimens (char const *file, int fd, struct timespec const 
timespec[2])

   if (adjustment_needed || (REPLACE_FUNC_STAT_FILE && fd < 0))
     {
-      struct stat st;
-      if (fd < 0 ? stat (file, &st) : fstat (fd, &st))
+      if (adjustment_needed != 3
+          && (fd < 0 ? stat (file, &st) : fstat (fd, &st)))
         return -1;
       if (ts && update_timespec (&st, &ts))
         return 0;
@@ -422,7 +424,6 @@ lutimens (char const *file, struct timespec const timespec
[2])
     {
       int result;
 # if __linux__
-      struct stat st;
       /* 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
@@ -439,6 +440,8 @@ lutimens (char const *file, struct timespec const timespec
[2])
             ts[0] = get_stat_atime (&st);
           else if (ts[1].tv_nsec == UTIME_OMIT)
             ts[1] = get_stat_mtime (&st);
+          /* Note that st is good, in case utimensat gives ENOSYS.  */
+          adjustment_needed++;
         }
 # endif /* __linux__ */
       result = utimensat (AT_FDCWD, file, ts, AT_SYMLINK_NOFOLLOW);
@@ -469,7 +472,7 @@ lutimens (char const *file, struct timespec const timespec
[2])

   if (adjustment_needed || REPLACE_FUNC_STAT_FILE)
     {
-      if (lstat (file, &st))
+      if (adjustment_needed != 3 && lstat (file, &st))
         return -1;
       if (ts && update_timespec (&st, &ts))
         return 0;
-- 
1.6.4.2