Tito wrote:
On Thursday 01 May 2014 21:30:25 Ralf Friedl wrote:
Tito wrote:
while trying to cleanup the debianutils/which command I've spotted
a minor glitch in bb's find_execable function as it is not able to handle
zero lenght prefixes in the PATH variable:
http://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap08.html
8.3 Other Environment Variables
PATH
snip
A zero-length prefix is a legacy feature that indicates the current
working directory. It appears as two adjacent colons ( "::" ), as an
initial colon preceding the rest of the list, or as a trailing colon
following the rest of the list. A strictly conforming application shall
use an actual pathname (such as .) to represent the current working
directory in PATH .
The real which command (really a script) in debianutils 4.4 does in fact
handle zero-length prefixes in PATH correctly:
for ELEMENT in $PATH; do
if [ -z "$ELEMENT" ]; then
ELEMENT=.
fi
So PATCH 1 actually fixes this, the size impact is huge
and therefore it is dependent on CONFIG_DEKTOP:
Why is the size impact so huge? Why don't you use the equivalent of the
script, if ELEMENT empty the ELEMENT="."?
Hi,
because it is tricky to perform the same thing that IFS=: does.
I think the line
if (*p == ':' && p[1] != '.')
is even wrong and should be
if (*p == ':')
No, if you remove && p[1] != '.' it will process
also a PATH=:: that were transformed to PATH=:.:
in the previous pass.
Actually it should do that. "PATH=::" is equivalent to "PATH=.:.:."
If you don't remove it, it won't process PATH=:./subdir
There is a similar problem for PATH=dirwithdot.:
What about this change:
while (p) {
n = strchr(p, ':');
if (n)
*n++ = '\0';
+ if (*p == '\0')
+ p = ".";
- if (*p != '\0') { /* it's not a PATH="foo::bar"
situation */
...
- }
The size impact should be the assignment 'p = "."' and the constant ".",
about 10 bytes total, no extra processing, no extra malloc.
I also thought it was that easy in the beginning, but if I recall it
correctly in this way it was not working. With your changes:
export PATH=:/usr/local/bin:/usr/bin:/bin::/usr/local/games:/usr/games:
debian:~/Desktop/SourceCode/busybox$ ./busybox which -a busybox
/bin/busybox
but it should show also 3 times the busybox binary in the cwd.
I don't know exactly what you did, but for me it works:
$ git diff
diff --git a/libbb/execable.c b/libbb/execable.c
index 178a00a..41540dc 100644
--- a/libbb/execable.c
+++ b/libbb/execable.c
@@ -37,6 +37,8 @@ char* FAST_FUNC find_execable(const char *filename,
char **PATHp)
n = strchr(p, ':');
if (n)
*n++ = '\0';
+ if (*p == '\0')
+ p = ".";
if (*p != '\0') { /* it's not a PATH="foo::bar"
situation */
p = concat_path_file(p, filename);
if (execable_file(p)) {
$ PATH=:/bin::/usr/bin: ./busybox which -a busybox
./busybox
./busybox
./busybox
To avoid a warning about "." being const, it should be written as
--- a/libbb/execable.c
+++ b/libbb/execable.c
@@ -37,14 +37,12 @@ char* FAST_FUNC find_execable(const char *filename,
char **PATHp)
n = strchr(p, ':');
if (n)
*n++ = '\0';
- if (*p != '\0') { /* it's not a PATH="foo::bar" situation */
- p = concat_path_file(p, filename);
+ p = concat_path_file(*p ? p : ".", filename);
if (execable_file(p)) {
*PATHp = n;
return p;
}
free(p);
- }
p = n;
} /* on loop exit p == NULL */
return p;
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
