Re: [PATCH] ls: print ?, not 0 as inode of dereferenced dangling symlink

2009-09-29 Thread Jim Meyering
Jim Meyering wrote:
 Here's another corner-case fix.
 I'll push something like this as soon as I've updated NEWS
 and added a test.

From 26a1306a0a9028eceed388dad0d8916aeeb00233 Mon Sep 17 00:00:00 2001
 From: Jim Meyering meyer...@redhat.com
 Date: Mon, 28 Sep 2009 20:24:41 +0200

Here's a more complete patch, though I still have
to add a mention in NEWS about the origin.


From f4d682808c6c8d2e1871278f3e81d0d0766f792b Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Mon, 28 Sep 2009 20:24:41 +0200
Subject: [PATCH] ls: print ?, not 0 as inode of dereferenced dangling 
symlink

ls prints inode numbers two ways: for long (-l) listings,
and for short ones, e.g., ls -li and ls -i.  The code to print
long listings properly printed ? when the inode was unknown,
but the code for handling short listings would print 0 instead.
Factor out the formatting code into a new function so ls prints
the right string (?) from both places:
* NEWS (Bug fixes): Mention it.
* src/ls.c (format_inode): New function.
(print_long_format): Use it here.
(print_file_name_and_frills): Use it here, too.
* tests/ls/dangle: Exercise this fix.
Reported by Yang Ren in http://bugzilla.redhat.com/525400
---
 NEWS|8 
 src/ls.c|   18 +-
 tests/ls/dangle |8 
 3 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/NEWS b/NEWS
index d0a9a7d..1a0c847 100644
--- a/NEWS
+++ b/NEWS
@@ -18,6 +18,14 @@ GNU coreutils NEWS-*- 
outline -*-

   ls -LR exits with status 2, not 0, when it encounters a cycle

+  ls -L now print ?, not 0 as the inode of a dangling symlink.
+  For example, before, mkdir d; ln -s no-such d/s; ls -Li d would print
+ls: cannot access d/s: No such file or directory
+0 s
+  now it prints this:
+ls: cannot access d/s: No such file or directory
+? s
+
 ** Portability

   On Solaris 9, many commands would mistakenly treat file/ the same as
diff --git a/src/ls.c b/src/ls.c
index 86f5c32..c8e8abb 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -3556,9 +3556,19 @@ format_group_width (gid_t g)
   return format_user_or_group_width (numeric_ids ? NULL : getgroup (g), g);
 }

+/* Return a pointer to a formatted version of F-stat.st_ino,
+   possibly using buffer, BUF, of length BUFLEN, which must be at least
+   INT_BUFSIZE_BOUND (uintmax_t) bytes.  */
+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));
+}

 /* Print information about F in long format.  */
-
 static void
 print_long_format (const struct fileinfo *f)
 {
@@ -3615,9 +3625,7 @@ print_long_format (const struct fileinfo *f)
 {
   char hbuf[INT_BUFSIZE_BOUND (uintmax_t)];
   sprintf (p, %*s , inode_number_width,
-   (f-stat.st_ino == NOT_AN_INODE_NUMBER
-? ?
-: umaxtostr (f-stat.st_ino, hbuf)));
+   format_inode (hbuf, sizeof hbuf, f));
   /* Increment by strlen (p) here, rather than by inode_number_width + 1.
  The latter is wrong when inode_number_width is zero.  */
   p += strlen (p);
@@ -4004,7 +4012,7 @@ print_file_name_and_frills (const struct fileinfo *f, 
size_t start_col)

   if (print_inode)
 printf (%*s , format == with_commas ? 0 : inode_number_width,
-umaxtostr (f-stat.st_ino, buf));
+format_inode (buf, sizeof buf, f));

   if (print_block_size)
 printf (%*s , format == with_commas ? 0 : block_size_width,
diff --git a/tests/ls/dangle b/tests/ls/dangle
index b2f8539..6abad92 100755
--- a/tests/ls/dangle
+++ b/tests/ls/dangle
@@ -26,6 +26,9 @@ fi
 ln -s no-such-file dangle || framework_failure
 mkdir -p dir/sub || framework_failure
 ln -s dir slink-to-dir || framework_failure
+mkdir d || framework_failure
+ln -s no-such d/dangle || framework_failure
+echo '? dangle'  subdir_exp || framework_failure

 fail=0

@@ -50,4 +53,9 @@ EOF

 compare out exp || fail=1

+# Ensure that ls -Li prints ? as the inode of a dangling symlink.
+rm -f out
+ls -Li d  out 2/dev/null  fail=1
+compare out subdir_exp || fail=1
+
 Exit $fail
--
1.6.5.rc2.177.ga9dd6




[PATCH] ls: print ?, not 0 as inode of dereferenced dangling symlink

2009-09-28 Thread Jim Meyering
Here's another corner-case fix.
I'll push something like this as soon as I've updated NEWS
and added a test.

From 26a1306a0a9028eceed388dad0d8916aeeb00233 Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Mon, 28 Sep 2009 20:24:41 +0200
Subject: [PATCH] ls: print ?, not 0 as inode of dereferenced dangling 
symlink

ls prints inode numbers two ways: for long (-l) listings,
and for short ones, e.g., ls -li and ls -i.  The code to print
long listings properly printed ? when the inode was unknown,
but the code for handling short listings would print 0 instead.
Factor out the formatting code into a new function so ls prints
the right string (?) from both places:
* src/ls.c (format_inode): New function.
(print_long_format): Use it here.
(print_file_name_and_frills): Use it here, too.
Reported by Yang Ren in http://bugzilla.redhat.com/525400
---
 src/ls.c |   18 +-
 1 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/src/ls.c b/src/ls.c
index 86f5c32..c8e8abb 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -3556,9 +3556,19 @@ format_group_width (gid_t g)
   return format_user_or_group_width (numeric_ids ? NULL : getgroup (g), g);
 }

+/* Return a pointer to a formatted version of F-stat.st_ino,
+   possibly using buffer, BUF, of length BUFLEN, which must be at least
+   INT_BUFSIZE_BOUND (uintmax_t) bytes.  */
+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));
+}

 /* Print information about F in long format.  */
-
 static void
 print_long_format (const struct fileinfo *f)
 {
@@ -3615,9 +3625,7 @@ print_long_format (const struct fileinfo *f)
 {
   char hbuf[INT_BUFSIZE_BOUND (uintmax_t)];
   sprintf (p, %*s , inode_number_width,
-   (f-stat.st_ino == NOT_AN_INODE_NUMBER
-? ?
-: umaxtostr (f-stat.st_ino, hbuf)));
+   format_inode (hbuf, sizeof hbuf, f));
   /* Increment by strlen (p) here, rather than by inode_number_width + 1.
  The latter is wrong when inode_number_width is zero.  */
   p += strlen (p);
@@ -4004,7 +4012,7 @@ print_file_name_and_frills (const struct fileinfo *f, 
size_t start_col)

   if (print_inode)
 printf (%*s , format == with_commas ? 0 : inode_number_width,
-umaxtostr (f-stat.st_ino, buf));
+format_inode (buf, sizeof buf, f));

   if (print_block_size)
 printf (%*s , format == with_commas ? 0 : block_size_width,
--
1.6.5.rc2.177.ga9dd6




Re: [PATCH] ls: print ?, not 0 as inode of dereferenced dangling symlink

2009-09-28 Thread Pádraig Brady
Jim Meyering wrote:
 Here's another corner-case fix.
 I'll push something like this as soon as I've updated NEWS
 and added a test.
 
From 26a1306a0a9028eceed388dad0d8916aeeb00233 Mon Sep 17 00:00:00 2001
 From: Jim Meyering meyer...@redhat.com
 Date: Mon, 28 Sep 2009 20:24:41 +0200
 Subject: [PATCH] ls: print ?, not 0 as inode of dereferenced dangling 
 symlink
 
 ls prints inode numbers two ways: for long (-l) listings,
 and for short ones, e.g., ls -li and ls -i.  The code to print
 long listings properly printed ? when the inode was unknown,
 but the code for handling short listings would print 0 instead.
 Factor out the formatting code into a new function so ls prints
 the right string (?) from both places

looks good.

For my reference I notice the documented `find` behavior is to
fall back to reporting on the dangling symlinks themselves:

$ ls -RLli t/
t/:
ls: cannot access t/bad.link: No such file or directory
total 0
? l? ? ?   ?   ?? bad.link
88141 -rw-rw-r-- 1 padraig padraig 0 2009-09-28 22:10 t
88141 -rw-rw-r-- 1 padraig padraig 0 2009-09-28 22:10 t.link

$ find -L t/ -ls
 846914 drwxrwxr-x   2 padraig  padraig  4096 Sep 28 22:13 t/
 881410 -rw-rw-r--   1 padraig  padraig 0 Sep 28 22:10 t/t
 881410 -rw-rw-r--   1 padraig  padraig 0 Sep 28 22:10 t/t.link
1348260 lrwxrwxrwx   1 padraig  padraig 4 Sep 28 22:04 t/bad.link 
- blah