Commit 1363: http://landley.net/hg/toybox/rev/1363
Ok, starting with toys/pending/mkpasswd.c: The is_salt_valid() function is only called from one place, and using a regex to check isalnum() or two punctuation characters is a bit overkill. (Since we did not use TOYFLAG_LOCALE we're in the C locale, and thus isalnum() has predictable behavior defined by the C99 spec, sections 7.4.1 and 5.2.1.1.) So zap that function and replace it with a for loop at the call site. We don't need "-m help" when we can put the list of supported types in the help text itself. (Unless this is used programmatically to autodetect support? The ubuntu version outputs a lot of extra verbiage that would make parsing hard, and the busybox-1.19.0 I have lying around doesn't support -m help at all?) I've yanked it for now, I can put -m help back if anybody's actually using it... This calls get_salt() which is a function in lib/password.c, a pending library function. So let's clean that up. get_salt: - Use libbuf instead of a local buf[] - The bit at the beginning with the if/else staircase can use a table. - Why is offset always 3? Even in the xase of algo="des" where we only added one $, we return an offset of 3. I'm going to assume that was a mistake. - We don't need to track "offset", just retain the initial salt starting point and use pointer subtraction. So new "char *s = salt;" - loop on ARRAY_LEN(), do the work in the loop, and return -1 if the loop exits. So, move the prototype from lib/portability.h to lib/lib.h to track which functions have been cleaned up. Back in mkpasswd.c, right after calling get_salt() we overwrite the returned salt with command line argument salt if we have it, so we read entropy from /dev/urandom but didn't use it. Given how often that's a finite resource on embedded devices this makes me uncomfortable, but I'll leave it for now... Why do we dup2() TT.pfd instead of using that as what we read from? If it was never set it'll be 0, which is stdin anyway. (Also, if you fed the old code -P 0 it would dup it, then close stdin. Oops?) Ah, I see: it's because read_password() doesn't let us control the fd it reads from, and we want -p to be able to specify a tty. (And here's _another_ pending library function I need to clean up... Eh, leave it for now.) Ok, change the FLAG_P test to if (TT.pfd) and then we don't close stdin if they go -P 0. Don't bother saying STDIN_FILENO, just use 0. You know, int offset and int i don't overlap in scope, just have the top one be int i. (And it doesn't need to be initialized because get_salt() assigns to offset as the first use. We don't need to strcpy the first argument to toybuf in the else case, just put an ? : in the call to crypt. So that's the cleanup on mkpasswd itself. Still have read_password() to do in lib/password.c, but other than that the command's ready to be promoted. (To "other" I guess, since it says "No standard" and thus presumably is not in LSB? Yeah, not in posix or LSB. Apparently it comes from the "whois" package, for no readily apparent reason...?) Rob _______________________________________________ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net