[PATCH 0/2] fix find_execable function and which code cleanup
Tito
farmatito at tiscali.it
Thu May 1 18:51:24 UTC 2014
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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 01_execable.patch
Type: text/x-patch
Size: 1419 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20140501/2079dab1/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 02_which.patch
Type: text/x-patch
Size: 2468 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20140501/2079dab1/attachment-0001.bin>
More information about the busybox
mailing list