Re: [PATCH] ls: don't use an undefined struct stat after failed stat/lstat

[Jim Meyering]

Eric Blake wrote:
> According to Jim Meyering on 9/29/2009 5:13 AM:
>> -          /* The failed stat/lstat may have modified f->stat.  Clear it,
>> -             since we may use at least its st_ino member, e.g.,
>> -             when trying to print the inode of dangling symlink:
>> -             mkdir d; ln -s no-such d/s; ls -Li d  */
>> -          memset (&f->stat, 0, sizeof (f->stat));
>> +          f->stat.st_ino = 22;
>
> Why 22?  Is this debugging leftovers?

Thanks for the review.
BTW, here's the merged version:

>From b7aaa0da8b47f4f373d3e0876bd540986278c6e2 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Tue, 29 Sep 2009 07:28:01 +0200
Subject: [PATCH] ls: don't use an undefined struct stat after failed stat/lstat
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* src/ls.c (format_inode): Access f->stat only if f->stat_ok is set.
* NEWS (Bug fixes): Mention it.
Improved-by: Pádraig Brady <address@hidden>
---
 NEWS     |    4 ++++
 src/ls.c |    6 +++---
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/NEWS b/NEWS
index 68ac24b..075c0fa 100644
--- a/NEWS
+++ b/NEWS
@@ -21,6 +21,10 @@ GNU coreutils NEWS                                    -*- 
outline -*-
   ls -Li is now consistent with ls -Lil in printing "?", not "0" as the
   inode of a dangling symlink.

+  ls -Li no longer relies on unspecified behavior of stat/lstat.
+  Before this change, "ls -Li dangling-symlink" would mistakenly
+  print the inode number of the symlink under some conditions.
+
 ** Portability

   On Solaris 9, many commands would mistakenly treat file/ the same as
diff --git a/src/ls.c b/src/ls.c
index c8e8abb..801e717 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -3563,9 +3563,9 @@ static char *
 format_inode (char *buf, size_t buflen, const struct fileinfo *f)
 {
   assert (INT_BUFSIZE_BOUND (uintmax_t) <= buflen);
-  return (f->stat.st_ino == NOT_AN_INODE_NUMBER
-          ? (char *) "?"
-          : umaxtostr (f->stat.st_ino, buf));
+  return (f->stat_ok && f->stat.st_ino != NOT_AN_INODE_NUMBER
+          ? umaxtostr (f->stat.st_ino, buf)
+          : (char *) "?");
 }

 /* Print information about F in long format.  */
--
1.6.5.rc2.177.ga9dd6