[BusyBox 0000007]: which and wd-located files

bugs at busybox.net bugs at busybox.net
Wed Sep 14 13:29:05 UTC 2005


A NOTE has been added to this issue. 
====================================================================== 
http://busybox.net/bugs/view.php?id=7 
====================================================================== 
Reported By:                felipek
Assigned To:                BusyBox
====================================================================== 
Project:                    BusyBox
Issue ID:                   7
Category:                   Standards Compliance
Reproducibility:            always
Severity:                   minor
Priority:                   normal
Status:                     assigned
====================================================================== 
Date Submitted:             01-13-2005 06:06 PST
Last Modified:              09-14-2005 06:29 PDT
====================================================================== 
Summary:                    which and wd-located files
Description: 
which doesn't search $PATH when there's a file in the WD with the same name
of the 'filename' parameter -- the "original which" behaves different. The
problem was reported by Brian T (24aa01c4d719$fa6d7c50$fd0ba8c0 at briantpc)
and fixed by me (Pine.LNX.4.61.0412010307520.25751 at nyvra.alien.bet).
Additional information includes my reply.
====================================================================== 

---------------------------------------------------------------------- 
 vsauder - 06-21-05 11:52  
---------------------------------------------------------------------- 
I also note that 'which' does not discern between executables and
directories. The access() method used is just a file/dir permission
checker. There are no provisions to determine that the path being checked
is a file.

Example:
# touch ls
# ls -l
-rw-r--r--    1 root     root            0 Jun 21 14:41 ls
# which ls
/bin/ls
# chmod a+x ls
# which ls
ls
# echo $PATH
/usr/sbin:/bin:/usr/bin:/sbin:.
# rm ls
# mkdir ls
# which ls
ls
# ls -l
drwxr-xr-x    2 root     root           40 Jun 21 14:44 ls

>From the which man page:
  Which takes one or more arguments. For each of its arguments it prints
  to stdout the full path of the executables that would have been exe-
  cuted when this argument had been entered at the shell prompt. 

It seems to me that even a minimalistic implementation should not show
directories. 

---------------------------------------------------------------------- 
 pgf - 07-18-05 15:12  
---------------------------------------------------------------------- 
I agree.  the behavior with respect to dirs should be fixed before this is
committed.  also, the original patch doesn't work correctly if PATH
contains
the current dir, via "::", i.e. "PATH=/bin:/usr/bin::/usr/local/bin". 

---------------------------------------------------------------------- 
 bernhardf - 08-31-05 06:24  
---------------------------------------------------------------------- 
with the busybox.which-fix.01b.diff, which behaves properly:
$ ./busybox which ls
/bin/ls
$ touch ls && chmod +x ls
$ ./busybox which ls
/bin/ls
$ rm -f ls ; mkdir ls
$ ./busybox which ls
/bin/ls

I don't have '.' in my PATH.
If someone can point me to the specification which forces '::' in the PATH
to be treated as ':.:' then i can change that too.
It sounds a bit error prone to me, though.

 

---------------------------------------------------------------------- 
 pgf - 08-31-05 05:54  
---------------------------------------------------------------------- 
i agree that having an empty directory represent "current" is error prone,
but it's certainly traditional -- SysV /bin/sh always did that, and a
little experimentation will show you that bash will too.  but you're right
that it's not really documented anymore.  it should be.  here's some
evidence: if you "man execvp", you'll see that the default PATH if none is
set is ":/bin:/usr/bin", which puts the current directory first.

also, in a sh man page here: 
http://www.neosoft.com/neosoft/man/sh.1.html
i found this text:

 "Path Search"

 "When locating a command, the shell first looks to see if it has a shell 
 function by that name. Then it looks for a builtin command by that name.
Finally, it searches each entry in PATH in turn for the command."

 "The value of the PATH variable should be a series of entries separated
by colons. Each entry consists of a directory name. The current directory
may be indicated by an empty directory name." 

---------------------------------------------------------------------- 
 bernhardf - 09-01-05 05:18  
---------------------------------------------------------------------- 
The busybox.which-fix.01c.diff that is attached does no longer peruse
is_directory() from libbb but rather stat's the given file to see if it's
a regular file. see Rob Landleys comment 
(http://busybox.net/lists/busybox/2005-August/015720.html)

Doing the access() call by hand was considerably bigger than simply
calling access().

thanks for applying. 

---------------------------------------------------------------------- 
 bernhardf - 09-09-05 06:51  
---------------------------------------------------------------------- 
I'm attaching a final  busybox.which-fix.01d.diff

a) This stat's the file and rejects directories.

b) It deals with ':' the same way as the "big" which(1) i have does:
":/one:/two" -> ".:/one:/two"
"::/one:/two" -> ".:/one:/two"
"/one::/two" -> "/one:.:/two"

ok to apply?
thanks, 

---------------------------------------------------------------------- 
 pgf - 09-12-05 07:20  
---------------------------------------------------------------------- 
hi bernhard -- looking at the patch, it's not clear to me that it takes
makes this:
   /bin:/usr/bin:
be the same as this:
   /bin:/usr/bin:.
does it?

you have comments (and code :-) for the other two cases (leading ':' and
double colon), but not for the trailing colon case.  but maybe it's taken
care of and i missed it when i skimmed the code.

paul 

---------------------------------------------------------------------- 
 bernhardf - 09-12-05 15:06  
---------------------------------------------------------------------- 
No, i was the one who missed that. Sorry :-/

Does that busybox.which-fix.01e.diff work for you? It did pass the few
tests i did (including the trailing ':'), fwiw. 

---------------------------------------------------------------------- 
 bernhardf - 09-13-05 09:19  
---------------------------------------------------------------------- 
I'm attaching a shrinked version (02a.diff);

$ size debianutils/which.o*
   text	   data	    bss	    dec	    hex	filename
    506	      0	      0	    506	    1fa	debianutils/which.o
    541	      0	      0	    541	    21d	debianutils/which.o.01e 

---------------------------------------------------------------------- 
 pgf - 09-14-05 06:29  
---------------------------------------------------------------------- 
version 2a looks good! 

Issue History 
Date Modified   Username       Field                    Change               
====================================================================== 
01-13-05 06:06  felipek        New Issue                                    
01-13-05 06:06  felipek        File Added: bb-which.patch                    
01-13-05 06:07  felipek        Issue Monitored: felipek                     
03-16-05 12:26  andersen       Assigned To              andersen => BusyBox 
06-21-05 11:39  vsauder        Issue Monitored: vsauder                     
06-21-05 11:52  vsauder        Note Added: 0000250                          
07-18-05 15:12  pgf            Note Added: 0000299                          
08-31-05 05:24  bernhardf      Note Added: 0000473                          
08-31-05 05:25  bernhardf      File Added: busybox.which-fix.01b.diff           
        
08-31-05 05:26  bernhardf      Note Edited: 0000473                         
08-31-05 05:54  pgf            Note Added: 0000474                          
08-31-05 06:24  bernhardf      Note Edited: 0000473                         
09-01-05 05:18  bernhardf      Note Added: 0000483                          
09-01-05 05:19  bernhardf      File Added: busybox.which-fix.01c.diff           
        
09-09-05 06:46  bernhardf      File Added: busybox.which-fix.01d.diff           
        
09-09-05 06:52  bernhardf      Note Added: 0000512                          
09-12-05 07:20  pgf            Note Added: 0000514                          
09-12-05 15:06  bernhardf      Note Added: 0000515                          
09-12-05 15:07  bernhardf      File Added: busybox.which-fix.01e.diff           
        
09-13-05 09:19  bernhardf      Note Added: 0000516                          
09-13-05 09:21  bernhardf      File Added: busybox.which-fix.02a.diff           
        
09-14-05 06:29  pgf            Note Added: 0000519                          
======================================================================




More information about the busybox-cvs mailing list