[Toybox] [CLEANUP] mkpasswd.c

Rob Landley rob at landley.net
Wed Jun 25 21:02:03 PDT 2014


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

 1403755323.0


More information about the Toybox mailing list