Hi,
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:
find_execable 81 182 +101
as parsing the PATH was not trivial as once you've started
it is hard to tell if a delimiter was a leading, trailing or double
colon.
I've tried and retried but no solution was satisfactory
as always some corner case was not parsed correctly,
so I changed strategy and normalize the PATH variable
so that it is easier to parse before starting the parsing.
Hints about a better solution with less code are welcome!!!
--- libbb/execable.c.orig 2013-06-02 13:56:34.000000000 +0200
+++ libbb/execable.c 2014-05-01 20:30:02.966512357 +0200
@@ -30,9 +30,37 @@
*/
char* FAST_FUNC find_execable(const char *filename, char **PATHp)
{
+ /*
http://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap08.html
+ * 8.3 Other Environment Variables - PATH
+ * 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.
+ */
+
+ IF_DESKTOP(int len;)
char *p, *n;
p = *PATHp;
+#if ENABLE_DESKTOP
+ /* Normalize PATH and add . for cwd where needed */
+ while ((n = strstr(p, "::")) != NULL) { /* Double :: */
+ *n = '\0';
+ n += 2;
+ p = xasprintf("%s:.:%s", p, n);
+ }
+
+ if (*p == ':' && p[1] != '.') { /* Leading single : */
+ p = xasprintf(".%s", p);
+ }
+
+ len = strlen(p) - 1;
+ if (p[len] == ':' && p[len - 1] != '.') { /* Trailing single : */
+ p = xasprintf("%s.:", p);
+ }
+
+ *PATHp = p;
+#endif
while (p) {
n = strchr(p, ':');
if (n)
The find execable_function and the exists_execable function
are used only in which and ifupdown and this change
seems not to affect them negatively.
PATCH 2 removes the #if ENABLE_DESKTOP part from
which relative to the -a option and unifies the code.
The size increase is negligible vs the version with
ENABLE_DESKTOP disabled:
scripts/bloat-o-meter busybox.old.nodesktop busybox_unstripped
function old new delta
which_main 193 202 +9
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 1/0 up/down: 9/0) Total: 9 bytes
So it makes sense to trade 9 bytes for the improved functionality
and reduced number of code lines.
In the end our which command looks like this:
int which_main(int argc UNUSED_PARAM, char **argv)
{
int found;
char *path;
char *p;
char *tmp;
int status = 0;
opt_complementary = "-1"; /* at least one argument */
getopt32(argv, "a");
argv += optind;
do {
found = 0;
/* If file contains a slash don't use PATH */
if (strchr(*argv, '/') && execable_file(*argv)) {
/* returns non-negative number on success, or EOF (-1)
on error */
found = puts(*argv);
} else {
p = getenv("PATH");
if (p == NULL) {
p = (char *)bb_default_root_path;
}
path = tmp = xstrdup(p);
while ((p = find_execable(*argv, &tmp)) != NULL) {
found = puts(p);
free(p);
if (!option_mask32) /* -a not set*/
break;
}
free(path);
}
status |= (!found);
} while (*(++argv) != NULL);
fflush_stdout_and_exit(status);
}
Hints, critics and improvements are welcome,
Ciao,
Tito
P.S.: the patches could be applied indipendently.
Find_execable function is not able to handle zero lenght prefixes
in the PATH variable as: two adjacent colons ( "::" ), an initial
colon preceding the rest of the list, or as a trailing colon
following the rest of the list.
Signed-off by Tito Ragusa <farmat...@tiscali.it>
--- libbb/execable.c.orig 2013-06-02 13:56:34.000000000 +0200
+++ libbb/execable.c 2014-05-01 20:30:02.966512357 +0200
@@ -30,9 +30,37 @@
*/
char* FAST_FUNC find_execable(const char *filename, char **PATHp)
{
+ /* http://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap08.html
+ * 8.3 Other Environment Variables - PATH
+ * 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.
+ */
+
+ IF_DESKTOP(int len;)
char *p, *n;
p = *PATHp;
+#if ENABLE_DESKTOP
+ /* Normalize PATH and add . for cwd where needed */
+ while ((n = strstr(p, "::")) != NULL) { /* Double :: */
+ *n = '\0';
+ n += 2;
+ p = xasprintf("%s:.:%s", p, n);
+ }
+
+ if (*p == ':' && p[1] != '.') { /* Leading single : */
+ p = xasprintf(".%s", p);
+ }
+
+ len = strlen(p) - 1;
+ if (p[len] == ':' && p[len - 1] != '.') { /* Trailing single : */
+ p = xasprintf("%s.:", p);
+ }
+
+ *PATHp = p;
+#endif
while (p) {
n = strchr(p, ':');
if (n)
This patch removes the #if ENABLE_DESKTOP part from
which relative to the -a option and unifies the code.
Signed-off by Tito Ragusa <farmat...@tiscali.it>
--- debianutils/which.c.orig 2013-06-02 13:56:34.000000000 +0200
+++ debianutils/which.c 2014-05-01 20:40:17.289525924 +0200
@@ -24,75 +24,37 @@
int which_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
int which_main(int argc UNUSED_PARAM, char **argv)
{
- IF_DESKTOP(int opt;)
- int status = EXIT_SUCCESS;
+ int found;
char *path;
char *p;
+ char *tmp;
+ int status = 0;
- opt_complementary = "-1"; /* at least one argument */
- IF_DESKTOP(opt =) getopt32(argv, "a");
+ opt_complementary = "-1"; /* at least one argument */
+ getopt32(argv, "a");
argv += optind;
-
- /* This matches what is seen on e.g. ubuntu.
- * "which" there is a shell script. */
- path = getenv("PATH");
- if (!path) {
- path = (char*)bb_PATH_root_path;
- putenv(path);
- path += 5; /* skip "PATH=" */
- }
-
+
do {
-#if ENABLE_DESKTOP
-/* Much bloat just to support -a */
- if (strchr(*argv, '/')) {
- if (execable_file(*argv)) {
- puts(*argv);
- continue;
- }
- status = EXIT_FAILURE;
+ found = 0;
+ /* If file contains a slash don't use PATH */
+ if (strchr(*argv, '/') && execable_file(*argv)) {
+ /* returns non-negative number on success, or EOF (-1) on error */
+ found = puts(*argv);
} else {
- char *path2 = xstrdup(path);
- char *tmp = path2;
-
- p = find_execable(*argv, &tmp);
- if (!p)
- status = EXIT_FAILURE;
- else {
- print:
- puts(p);
- free(p);
- if (opt) {
- /* -a: show matches in all PATH components */
- if (tmp) {
- p = find_execable(*argv, &tmp);
- if (p)
- goto print;
- }
- }
- }
- free(path2);
- }
-#else
-/* Just ignoring -a */
- if (strchr(*argv, '/')) {
- if (execable_file(*argv)) {
- puts(*argv);
- continue;
+ p = getenv("PATH");
+ if (p == NULL) {
+ p = (char *)bb_default_root_path;
}
- } else {
- char *path2 = xstrdup(path);
- char *tmp = path2;
- p = find_execable(*argv, &tmp);
- free(path2);
- if (p) {
- puts(p);
+ path = tmp = xstrdup(p);
+ while ((p = find_execable(*argv, &tmp)) != NULL) {
+ found = puts(p);
free(p);
- continue;
+ if (!option_mask32) /* -a not set*/
+ break;
}
+ free(path);
}
- status = EXIT_FAILURE;
-#endif
+ status |= (!found);
} while (*(++argv) != NULL);
fflush_stdout_and_exit(status);
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
