Notes |
(0000250)
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. |
| |
(0000299)
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". |
| |
(0000473)
bernhardf
08-31-05 05:24
edited on: 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.
|
| |
(0000474)
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." |
| |
(0000483)
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. |
| |
(0000512)
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, |
| |
(0000514)
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 |
| |
(0000515)
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. |
| |
(0000516)
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 |
| |
(0000519)
pgf
09-14-05 06:29
|
version 2a looks good! |
| |
(0000521)
bernhardf
09-14-05 06:59
|
pgf,
I do not have svn write access. Please check it in *and* set the issue to resolved or closed.
thanks, |
| |
(0000522)
pgf
09-14-05 07:09
|
Committed revision 11454. |
| |
bb-which.patch [^] (642 bytes) 01-13-05 06:06
