NetBSD Problem Report #43751

From heas@sea.shrubbery.net  Thu Aug 12 18:52:44 2010
Return-Path: <heas@sea.shrubbery.net>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by www.NetBSD.org (Postfix) with ESMTP id D3F0963B883
	for <gnats-bugs@gnats.NetBSD.org>; Thu, 12 Aug 2010 18:52:44 +0000 (UTC)
Message-Id: <20100812165246.51A7C24B5EF@sea.shrubbery.net>
Date: Thu, 12 Aug 2010 16:52:46 +0000 (UTC)
From: heas@sea.shrubbery.net
Reply-To: heas@sea.shrubbery.net
To: gnats-bugs@gnats.NetBSD.org
Subject: ls -L is broken
X-Send-Pr-Version: 3.95

>Number:         43751
>Category:       bin
>Synopsis:       ls -L returns wrong exit code
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    bin-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Aug 12 18:55:00 +0000 2010
>Last-Modified:  Fri Aug 13 23:00:04 +0000 2010
>Originator:     heas@sea.shrubbery.net
>Release:        NetBSD 5.99.37
>Organization:

>Environment:


System: NetBSD sea 5.99.37 NetBSD 5.99.37 (GENERIC) #3: Wed Jul 28 19:07:07 UTC 2010 root@roome:/sys/arch/amd64/compile/GENERIC amd64
Architecture: x86_64
Machine: amd64
>Description:
ls -L should return non-zero exit code if the target of a symlink does
not exist.  I think this changed within a month or so.
>How-To-Repeat:
netbsd% ln -s /nonexistent 
netbsd% ls -L nonexistent
nonexistent
netbsd% echo $?
0
netbsd% ls /nonexistent 
ls: /nonexistent: No such file or directory

netbsd4.99% ln -s /nonexistent
netbsd4.99% ls -L !$
ls -L /nonexistent
ls: /nonexistent: No such file or directory
netbsd4.99% echo $?
1

solaris% ln -s /nonexistent
solaris% ls -L /nonexistent
ls: cannot access /nonexistent: No such file or directory
solaris% echo $?
2

>Fix:
Haven't looked into it yet.  Recent changes to src/bin/ls seem innocuous.

>Audit-Trail:
From: "Valeriy E. Ushakov" <uwe@stderr.spb.ru>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: bin/43751: ls -L returns wrong exit code
Date: Thu, 12 Aug 2010 23:18:04 +0400

 I guess you mean

     $ ls -L nonexistent

 not

     $ ls -L /nonexistent

 in your netbsd4.99 and solaris examples

 -uwe

From: "Valeriy E. Ushakov" <uwe@stderr.spb.ru>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: bin/43751: ls -L returns wrong exit code
Date: Thu, 12 Aug 2010 23:30:02 +0400

 I don't think this is recent.  NetBSD 1.6 behaves the same way.

 Basically, we use -L only to select FTS_LOGICAL instead of
 FTS_PHYSICAL.  printing code doesn't know whether -L was specified.
 For links with non-existent targets fts_children(3) returns the link
 info (as it would for FTS_PHYSICAL) and printing code just happily
 prints it.

 -uwe

From: john heasley <heas@shrubbery.net>
To: gnats-bugs@NetBSD.org
Cc: gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, heas@shrubbery.net
Subject: Re: bin/43751: ls -L returns wrong exit code
Date: Thu, 12 Aug 2010 18:17:29 -0700

 Thu, Aug 12, 2010 at 08:50:04PM +0000, Valeriy E. Ushakov:
 >  I don't think this is recent.  NetBSD 1.6 behaves the same way.

 Crap, you are right.  my 4.99 test had the FQPN, as you noted.

 >  Basically, we use -L only to select FTS_LOGICAL instead of
 >  FTS_PHYSICAL.  printing code doesn't know whether -L was specified.
 >  For links with non-existent targets fts_children(3) returns the link
 >  info (as it would for FTS_PHYSICAL) and printing code just happily
 >  prints it.

 Does it need to?  something like this is a quick fix.

 Index: ls.c
 ===================================================================
 RCS file: /cvsroot/src/bin/ls/ls.c,v
 retrieving revision 1.67
 diff -d -u -d -u -r1.67 ls.c
 --- ls.c	8 Jul 2010 20:43:34 -0000	1.67
 +++ ls.c	13 Aug 2010 00:52:59 -0000
 @@ -389,6 +389,7 @@
  static void
  traverse(int argc, char *argv[], int options)
  {
 +	char path[MAXPATHLEN + 1];
  	FTS *ftsp;
  	FTSENT *p, *chp;
  	int ch_options, error;
 @@ -442,6 +443,13 @@
  			if (!f_recursive && chp != NULL)
  				(void)fts_set(ftsp, p, FTS_SKIP);
  			break;
 +		case FTS_SLNONE:
 +			if (options & FTS_LOGICAL) {
 +			    readlink(p->fts_path, path, sizeof(path) - 1);
 +			    errno = ENOENT;
 +			    warn("cannot access %s", path, p->fts_name);
 +			    rval = EXIT_FAILURE;
 +			}
  		}
  	error = errno;
  	(void)fts_close(ftsp);

From: "Valeriy E. Ushakov" <uwe@stderr.spb.ru>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: bin/43751: ls -L returns wrong exit code
Date: Fri, 13 Aug 2010 06:42:36 +0400

 On Thu, Aug 12, 2010 at 18:17:29 -0700, john heasley wrote:

 > Thu, Aug 12, 2010 at 08:50:04PM +0000, Valeriy E. Ushakov:
 > 
 > Crap, you are right.  my 4.99 test had the FQPN, as you noted.
 > 
 > >  Basically, we use -L only to select FTS_LOGICAL instead of
 > >  FTS_PHYSICAL.  printing code doesn't know whether -L was specified.
 > >  For links with non-existent targets fts_children(3) returns the link
 > >  info (as it would for FTS_PHYSICAL) and printing code just happily
 > >  prints it.
 > 
 > Does it need to?  something like this is a quick fix.
 > 
 > Index: ls.c
 > ===================================================================
 > RCS file: /cvsroot/src/bin/ls/ls.c,v
 > retrieving revision 1.67
 > diff -d -u -d -u -r1.67 ls.c
 > --- ls.c	8 Jul 2010 20:43:34 -0000	1.67
 > +++ ls.c	13 Aug 2010 00:52:59 -0000
 > @@ -389,6 +389,7 @@
 >  static void
 >  traverse(int argc, char *argv[], int options)
 >  {
 > +	char path[MAXPATHLEN + 1];
 >  	FTS *ftsp;
 >  	FTSENT *p, *chp;
 >  	int ch_options, error;
 > @@ -442,6 +443,13 @@
 >  			if (!f_recursive && chp != NULL)
 >  				(void)fts_set(ftsp, p, FTS_SKIP);
 >  			break;
 > +		case FTS_SLNONE:
 > +			if (options & FTS_LOGICAL) {
 > +			    readlink(p->fts_path, path, sizeof(path) - 1);
 > +			    errno = ENOENT;
 > +			    warn("cannot access %s", path, p->fts_name);
 > +			    rval = EXIT_FAILURE;
 > +			}
 >  		}
 >  	error = errno;
 >  	(void)fts_close(ftsp);

 Shouldn't this check be in display()?

 You don't have to readlink(), the error is reported for the link name,
 not the target name.

 -uwe

From: john heasley <heas@shrubbery.net>
To: gnats-bugs@NetBSD.org
Cc: gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, heas@sea.shrubbery.net
Subject: Re: bin/43751: ls -L returns wrong exit code
Date: Fri, 13 Aug 2010 09:54:00 -0700

 Fri, Aug 13, 2010 at 02:45:01AM +0000, Valeriy E. Ushakov:
 >  Shouldn't this check be in display()?

 rather than print the file & error?  ie: only the error.  as below.

 >  You don't have to readlink(), the error is reported for the link name,
 >  not the target name.

 thats fine too.

 Index: ls.c
 ===================================================================
 RCS file: /cvsroot/src/bin/ls/ls.c,v
 retrieving revision 1.67
 diff -d -u -d -u -r1.67 ls.c
 --- ls.c	8 Jul 2010 20:43:34 -0000	1.67
 +++ ls.c	13 Aug 2010 16:52:01 -0000
 @@ -68,7 +68,7 @@
  #include "ls.h"
  #include "extern.h"

 -static void	 display(FTSENT *, FTSENT *);
 +static void	 display(FTSENT *, FTSENT *, int);
  static int	 mastercmp(const FTSENT **, const FTSENT **);
  static void	 traverse(int, char **, int);

 @@ -397,7 +397,7 @@
  	    fts_open(argv, options, f_nosort ? NULL : mastercmp)) == NULL)
  		err(EXIT_FAILURE, NULL);

 -	display(NULL, fts_children(ftsp, 0));
 +	display(NULL, fts_children(ftsp, 0), options);
  	if (f_listdir) {
  		(void)fts_close(ftsp);
  		return;
 @@ -437,7 +437,7 @@
  			}

  			chp = fts_children(ftsp, ch_options);
 -			display(p, chp);
 +			display(p, chp, options);

  			if (!f_recursive && chp != NULL)
  				(void)fts_set(ftsp, p, FTS_SKIP);
 @@ -456,7 +456,7 @@
   * points to the parent directory of the display list.
   */
  static void
 -display(FTSENT *p, FTSENT *list)
 +display(FTSENT *p, FTSENT *list, int options)
  {
  	struct stat *sp;
  	DISPLAY d;
 @@ -501,6 +501,13 @@
  			rval = EXIT_FAILURE;
  			continue;
  		}
 +		if (cur->fts_info == FTS_SLNONE && (options & FTS_LOGICAL)) {
 +			warnx("cannot access %s: %s", cur->fts_name,
 +			    strerror(ENOENT));
 +			cur->fts_number = NO_PRINT;
 +			rval = EXIT_FAILURE;
 +			continue;
 +		}

  		/*
  		 * P is NULL if list is the argv list, to which different rules

From: christos@zoulas.com (Christos Zoulas)
To: john heasley <heas@shrubbery.net>, gnats-bugs@NetBSD.org
Cc: gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, heas@sea.shrubbery.net
Subject: Re: bin/43751: ls -L returns wrong exit code
Date: Fri, 13 Aug 2010 12:55:46 -0400

 On Aug 13,  9:54am, heas@shrubbery.net (john heasley) wrote:
 -- Subject: Re: bin/43751: ls -L returns wrong exit code

 | rather than print the file & error?  ie: only the error.  as below.

 It is better to set errno and use warn, rather than warnx and strerror.

 christos

From: john heasley <heas@shrubbery.net>
To: Christos Zoulas <christos@zoulas.com>
Cc: john heasley <heas@shrubbery.net>, gnats-bugs@NetBSD.org,
	gnats-admin@netbsd.org, netbsd-bugs@netbsd.org,
	heas@sea.shrubbery.net
Subject: Re: bin/43751: ls -L returns wrong exit code
Date: Fri, 13 Aug 2010 10:12:11 -0700

 Fri, Aug 13, 2010 at 12:55:46PM -0400, Christos Zoulas:
 > On Aug 13,  9:54am, heas@shrubbery.net (john heasley) wrote:
 > -- Subject: Re: bin/43751: ls -L returns wrong exit code
 > 
 > | rather than print the file & error?  ie: only the error.  as below.
 > 
 > It is better to set errno and use warn, rather than warnx and strerror.
 > 

 Index: ls.c
 ===================================================================
 RCS file: /cvsroot/src/bin/ls/ls.c,v
 retrieving revision 1.67
 diff -d -u -d -u -r1.67 ls.c
 --- ls.c	8 Jul 2010 20:43:34 -0000	1.67
 +++ ls.c	13 Aug 2010 17:11:37 -0000
 @@ -68,7 +68,7 @@
  #include "ls.h"
  #include "extern.h"

 -static void	 display(FTSENT *, FTSENT *);
 +static void	 display(FTSENT *, FTSENT *, int);
  static int	 mastercmp(const FTSENT **, const FTSENT **);
  static void	 traverse(int, char **, int);

 @@ -397,7 +397,7 @@
  	    fts_open(argv, options, f_nosort ? NULL : mastercmp)) == NULL)
  		err(EXIT_FAILURE, NULL);

 -	display(NULL, fts_children(ftsp, 0));
 +	display(NULL, fts_children(ftsp, 0), options);
  	if (f_listdir) {
  		(void)fts_close(ftsp);
  		return;
 @@ -437,7 +437,7 @@
  			}

  			chp = fts_children(ftsp, ch_options);
 -			display(p, chp);
 +			display(p, chp, options);

  			if (!f_recursive && chp != NULL)
  				(void)fts_set(ftsp, p, FTS_SKIP);
 @@ -456,7 +456,7 @@
   * points to the parent directory of the display list.
   */
  static void
 -display(FTSENT *p, FTSENT *list)
 +display(FTSENT *p, FTSENT *list, int options)
  {
  	struct stat *sp;
  	DISPLAY d;
 @@ -501,6 +501,13 @@
  			rval = EXIT_FAILURE;
  			continue;
  		}
 +		if (cur->fts_info == FTS_SLNONE && (options & FTS_LOGICAL)) {
 +			errno = ENOENT;
 +			warnx("cannot access %s", cur->fts_name);
 +			cur->fts_number = NO_PRINT;
 +			rval = EXIT_FAILURE;
 +			continue;
 +		}

  		/*
  		 * P is NULL if list is the argv list, to which different rules

From: "Valeriy E. Ushakov" <uwe@stderr.spb.ru>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: bin/43751: ls -L returns wrong exit code
Date: Fri, 13 Aug 2010 22:22:49 +0400

 On Fri, Aug 13, 2010 at 10:12:11 -0700, john heasley wrote:

 > -static void	 display(FTSENT *, FTSENT *);
 > +static void	 display(FTSENT *, FTSENT *, int);

 Instead of passing options around, just introduce a global variable
 for the -L flag.

 Might be a good time to add -H support too.

 -uwe

From: john heasley <heas@shrubbery.net>
To: gnats-bugs@NetBSD.org
Cc: gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, heas@sea.shrubbery.net
Subject: Re: bin/43751: ls -L returns wrong exit code
Date: Fri, 13 Aug 2010 11:37:11 -0700

 Fri, Aug 13, 2010 at 06:25:02PM +0000, Valeriy E. Ushakov:
 > The following reply was made to PR bin/43751; it has been noted by GNATS.
 > 
 > From: "Valeriy E. Ushakov" <uwe@stderr.spb.ru>
 > To: gnats-bugs@NetBSD.org
 > Cc: 
 > Subject: Re: bin/43751: ls -L returns wrong exit code
 > Date: Fri, 13 Aug 2010 22:22:49 +0400
 > 
 >  On Fri, Aug 13, 2010 at 10:12:11 -0700, john heasley wrote:
 >  
 >  > -static void	 display(FTSENT *, FTSENT *);
 >  > +static void	 display(FTSENT *, FTSENT *, int);
 >  
 >  Instead of passing options around, just introduce a global variable
 >  for the -L flag.

 ok, after that, you're ok with this change?

 >  Might be a good time to add -H support too.

 meaning this (from solaris)?

      -H           If an argument is a symbolic link  that  refer-
                   ences  a  directory,  this option evaluates the
                   file information and file type of the directory
                   that  the link references, rather than those of
                   the link itself. However, the name of the  link
                   is displayed, rather than the referenced direc-
                   tory.

From: christos@zoulas.com (Christos Zoulas)
To: gnats-bugs@NetBSD.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, 
	heas@sea.shrubbery.net
Cc: 
Subject: Re: bin/43751: ls -L returns wrong exit code
Date: Fri, 13 Aug 2010 15:27:08 -0400

 On Aug 13,  5:15pm, heas@shrubbery.net (john heasley) wrote:
 -- Subject: Re: bin/43751: ls -L returns wrong exit code

 |  > It is better to set errno and use warn, rather than warnx and strerror.

 warn, not warnx.

 christos

From: "Valeriy E. Ushakov" <uwe@stderr.spb.ru>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: bin/43751: ls -L returns wrong exit code
Date: Sat, 14 Aug 2010 00:08:47 +0400

 On Fri, Aug 13, 2010 at 11:37:11 -0700, john heasley wrote:

 > Fri, Aug 13, 2010 at 06:25:02PM +0000, Valeriy E. Ushakov:
 > > The following reply was made to PR bin/43751; it has been noted by GNATS.
 > >  On Fri, Aug 13, 2010 at 10:12:11 -0700, john heasley wrote:
 > >  
 > >  > -static void	 display(FTSENT *, FTSENT *);
 > >  > +static void	 display(FTSENT *, FTSENT *, int);
 > >  
 > >  Instead of passing options around, just introduce a global variable
 > >  for the -L flag.
 > 
 > ok, after that, you're ok with this change?
 >
 > >  Might be a good time to add -H support too.
 > 
 > meaning this (from solaris)?
 > 
 >      -H           If an argument is a symbolic link  that  refer-
 >                   ences  a  directory,  this option evaluates the
 >                   file information and file type of the directory
 >                   that  the link references, rather than those of
 >                   the link itself. However, the name of the  link
 >                   is displayed, rather than the referenced direc-
 >                   tory.

 Yes.  Or this:

   http://www.opengroup.org/onlinepubs/009695399/utilities/ls.html

 Hmm, actually... From my cursory glance through the code -L vs. -H
 should translate into different "options" for command line args, so
 may be passing "options" (or at least something that indicates logical
 vs. physical) to display() is a better approach.

 Thanks for bearing with my drive-by nitpicking...

 -uwe

From: matthew green <mrg@eterna.com.au>
To: gnats-bugs@NetBSD.org
Cc: gnats-admin@netbsd.org, netbsd-bugs@netbsd.org,
    heas@sea.shrubbery.net
Subject: re: bin/43751: ls -L returns wrong exit code
Date: Sat, 14 Aug 2010 08:58:45 +1000

 one thing to be aware of is that some of the ls(1) code is shared
 with other parts of the source tree.  please check a full build
 works if you're changing ls much..

 thanks.


 .mrg.

>Unformatted:

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2007 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.