Re: [PATCH] ls: with -LR, exit with status 2 upon detecting a cycle

2009-09-29 Thread Jim Meyering
Pádraig Brady wrote:
 Jim Meyering wrote:
 Jim Meyering wrote:
 Here's a fix to make ls -LR work the same way Solaris 10's /bin/ls
 does in the presence of a cycle.

 It makes sense to exit(2) along with the printed error.
 The patch looks good.

Thanks for the review.




[PATCH] ls: with -LR, exit with status 2 upon detecting a cycle

2009-09-28 Thread Jim Meyering
Here's a fix to make ls -LR work the same way Solaris 10's /bin/ls
does in the presence of a cycle.

From fb245d0be6231f2962d1481d032b77f3211b29de Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Mon, 28 Sep 2009 18:29:02 +0200
Subject: [PATCH] ls: with -LR, exit with status 2 upon detecting a cycle

* src/ls.c (print_dir): Diagnosing the cycle is not enough.
Also set exit status to 2.  This is what Solaris' /bin/ls does, too.
* tests/ls/infloop: Rework test: match both expected stdout and stderr.
Require an exit status of 2 in this case.
* NEWS (Bug fixes): Mention it.
Reported by Yang Ren in http://bugzilla.redhat.com/525402.
---
 NEWS |2 ++
 src/ls.c |1 +
 tests/ls/infloop |   25 +
 3 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/NEWS b/NEWS
index 5060502..d0a9a7d 100644
--- a/NEWS
+++ b/NEWS
@@ -16,6 +16,8 @@ GNU coreutils NEWS-*- 
outline -*-
   when the color of a more specific type is disabled.
   [bug introduced in coreutils-5.90]

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

   On Solaris 9, many commands would mistakenly treat file/ the same as
diff --git a/src/ls.c b/src/ls.c
index 1bb6873..86f5c32 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -2471,6 +2471,7 @@ print_dir (char const *name, char const *realname, bool 
command_line_arg)
   error (0, 0, _(%s: not listing already-listed directory),
  quotearg_colon (name));
   closedir (dirp);
+  set_exit_status (true);
   return;
 }

diff --git a/tests/ls/infloop b/tests/ls/infloop
index 419d7a9..b77f88c 100755
--- a/tests/ls/infloop
+++ b/tests/ls/infloop
@@ -1,7 +1,7 @@
 #!/bin/sh
 # show that the following no longer makes ls infloop
 # mkdir loop; cd loop; ln -s ../loop sub; ls -RL
-
+# Also ensure ls exits with status = 2 in that case.
 # Copyright (C) 2001-2002, 2004, 2006-2009 Free Software Foundation, Inc.

 # This program is free software: you can redistribute it and/or modify
@@ -27,21 +27,22 @@ fi
 mkdir loop || framework_failure
 ln -s ../loop loop/sub || framework_failure

-fail=0
-
-ls -RL loop 2err | head -n 7  out
-# With an inf-looping ls, out will contain these 7 lines:
-cat EOF  bad
+cat \EOF  exp-out || framework_failure
 loop:
 sub
+EOF

-loop/sub:
-sub
-
-loop/sub/sub:
+cat \EOF  exp-err || framework_failure
+ls: loop/sub: not listing already-listed directory
 EOF

-# Make sure we don't get the bad output.
-compare out bad  /dev/null 21  fail=1
+fail=0
+
+timeout 1 ls -RL loop 2err  out
+# Ensure that ls exits with status 2 upon detecting a cycle
+test $? = 2 || fail=1
+
+compare err exp-err || fail=1
+compare out exp-out || fail=1

 Exit $fail
--
1.6.5.rc2.177.ga9dd6




Re: [PATCH] ls: with -LR, exit with status 2 upon detecting a cycle

2009-09-28 Thread Jim Meyering
Jim Meyering wrote:
 Here's a fix to make ls -LR work the same way Solaris 10's /bin/ls
 does in the presence of a cycle.

I've tweaked the patch to udpate the documentation, too.

From 9fad0c69f218a57c9404f7c1137961eec5a818f1 Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Mon, 28 Sep 2009 18:29:02 +0200
Subject: [PATCH] ls: with -LR, exit with status 2 upon detecting a cycle

* src/ls.c (print_dir): Diagnosing the cycle is not enough.
Also set exit status to 2.  This is what Solaris' /bin/ls does, too.
* tests/ls/infloop: Rework test: match both expected stdout and stderr.
Require an exit status of 2 in this case.
* doc/coreutils.texi (ls invocation): Mention that a loop provokes
in an exit status of 2.
* NEWS (Bug fixes): Mention it.
Reported by Yang Ren in http://bugzilla.redhat.com/525402.
---
 NEWS   |2 ++
 doc/coreutils.texi |5 +++--
 src/ls.c   |1 +
 tests/ls/infloop   |   25 +
 4 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/NEWS b/NEWS
index 5060502..d0a9a7d 100644
--- a/NEWS
+++ b/NEWS
@@ -16,6 +16,8 @@ GNU coreutils NEWS-*- 
outline -*-
   when the color of a more specific type is disabled.
   [bug introduced in coreutils-5.90]

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

   On Solaris 9, many commands would mistakenly treat file/ the same as
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index b10cc10..5026e76 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -6113,8 +6113,9 @@ ls invocation
 1 minor problems  (e.g., failure to access a file or directory not
   specified as a command line argument.  This happens when listing a
   directory in which entries are actively being removed or renamed.)
-2 serious trouble (e.g., memory exhausted, invalid option or failure
-  to access file or directory specified as a command line argument)
+2 serious trouble (e.g., memory exhausted, invalid option, failure
+  to access a file or directory specified as a command line argument
+  or a directory loop)
 @end display

 Also see @ref{Common options}.
diff --git a/src/ls.c b/src/ls.c
index 1bb6873..86f5c32 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -2471,6 +2471,7 @@ print_dir (char const *name, char const *realname, bool 
command_line_arg)
   error (0, 0, _(%s: not listing already-listed directory),
  quotearg_colon (name));
   closedir (dirp);
+  set_exit_status (true);
   return;
 }

diff --git a/tests/ls/infloop b/tests/ls/infloop
index 419d7a9..b77f88c 100755
--- a/tests/ls/infloop
+++ b/tests/ls/infloop
@@ -1,7 +1,7 @@
 #!/bin/sh
 # show that the following no longer makes ls infloop
 # mkdir loop; cd loop; ln -s ../loop sub; ls -RL
-
+# Also ensure ls exits with status = 2 in that case.
 # Copyright (C) 2001-2002, 2004, 2006-2009 Free Software Foundation, Inc.

 # This program is free software: you can redistribute it and/or modify
@@ -27,21 +27,22 @@ fi
 mkdir loop || framework_failure
 ln -s ../loop loop/sub || framework_failure

-fail=0
-
-ls -RL loop 2err | head -n 7  out
-# With an inf-looping ls, out will contain these 7 lines:
-cat EOF  bad
+cat \EOF  exp-out || framework_failure
 loop:
 sub
+EOF

-loop/sub:
-sub
-
-loop/sub/sub:
+cat \EOF  exp-err || framework_failure
+ls: loop/sub: not listing already-listed directory
 EOF

-# Make sure we don't get the bad output.
-compare out bad  /dev/null 21  fail=1
+fail=0
+
+timeout 1 ls -RL loop 2err  out
+# Ensure that ls exits with status 2 upon detecting a cycle
+test $? = 2 || fail=1
+
+compare err exp-err || fail=1
+compare out exp-out || fail=1

 Exit $fail
--
1.6.5.rc2.177.ga9dd6