Hello,

The following code snippet which was added by the U-Boot team to busybox' hush shell seems a bit strange. From u-boot-1.1.5/common/hush.c:

int parse_string_outer(char *s, int flag) { struct in_str input; #ifdef __U_BOOT__ char *p = NULL; int rcode; if ( !s || !*s) return 1; if (!(p = strchr(s, '\n')) || *++p) { p = xmalloc(strlen(s) + 2); strcpy(p, s); strcat(p, "\n"); setup_string_in_str(&input, p); rcode = parse_stream_outer(&input, flag); free(p); return rcode; } else { #endif setup_string_in_str(&input, s); return parse_stream_outer(&input, flag); #ifdef __U_BOOT__ } #endif }

It looks like the second if() clause was added to make sure that the passed string s is terminated by \n\0. However, this has some flaws:

1) If the string contains a \n which is not followed by a \0, the code tries to fix it by adding a \0. But strlen, strcpy and strcat work on zero-terminated strings. As it is, the code replaces the first \0 in (or beyond) the string with \n\0.

2) If 1) would be fixed, the function could not process strings containing more than one \n anymore (because a \0 would be added after the first \n).

3) If the string contains no \n (but is zero-terminated), the code inserts a \n before the \0. This messes up the return value of parse_stream_outer which scans for commands (everything which is terminated by either \n or \0), but returns the exit status of the last command only. If string s is terminated by \n\0, the last command is empty and parse_stream_outer always returns 0.

Problem 3) is the cause why autoscripts (both the function and the command) always return an exit value of 0.

In my opinion, the above check should be removed completely because the parameter string s should always be zero-terminated (all functions calling parse_string_outer already seem to ensure this). And, if you want the function to process the first command (terminated by \n or \0) only, the parameter flag should have bit FLAG_EXIT_FROM_LOOP set (like the commands 'boot' and 'run' do, which may be considered another bug..).

What do you think? Does anyone know the author of the above code snippet?

Best Regards, Daniel