[U-Boot-Users] Hush and Autoscript Bug
Daniel Hobi
dhobi at gmx.net
Wed Nov 15 12:33:37 CET 2006
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
More information about the U-Boot
mailing list