[PATCH] passwd size reduction and request for testing

Rob Landley rob at landley.net
Fri Jan 13 00:31:35 UTC 2006


On Thursday 12 January 2006 16:00, Tito wrote:
> Hi to all,
> this  patch reduces size of the passwd applet.
> It is based on hints and work of:
> Vladimir Oleynik,
> Mike Frysinger,
> Bernard Fischer,
> Roberto A. Foglietta.

Please don't say "(C) many different people", because that's not a _useful_ 
notice.  How about you say (C) you if you want to maintain it, and then 
"based on work by (above list) and others."

Why is "New password: " an insufficient prompt?  Why a paragraph there?  And 
why have a #define for this when it's only used once, anyway?

Maximum 8 characters went out with the linux 2.2 kernel, didn't it?  Your 
bufsize is 129.  (And the normal minimum is 6, not 5.)

At the start of copy_passwd_file(), having taken the liberty of simplifying 
the #ifdef out of the following code:

    rewind(fp);
    if ((out_fp = bb_wfopen(dest, "w"))
        || !chmod(dest, ENABLE_FEATURE_SHADOWPASSWDS ? 0600 : 0644)
        || !chown(dest, 0, 0)) {
        rewind(fp);

My first question is why do you rewind _again_ right after the if statement?

My secont question is what the heck is this _doing_...  The || means that the 
second and third bits will only trigger if the first _didn't_ trigger.  Don't 
you mean &&?  We'll happily continue if we couldn't open but we _could_ chmod 
the file (permission 000 can do that), in which case the if is running with a 
null out_fp...

The next few lines are a bit confusing too:

 while ((pw_rest = buffer = bb_get_line_from_file(fp)) != NULL) {
            if (username && strncmp(username, buffer, strlen(username)) == 0) 
{
                /* We have a match. */
                while(*pw_rest++ != ':'); /* move past username */

So if you have two users named "fred" and "frederick", and you tell this thing 
to find "fred", it'll match "frederick" if that comes first in the password 
list?  (Or do you have a ":" at the end of the username argument, in which 
case you _need_ a comment about that.)

I think I'm going to need a _lot_ more caffeine to properly audit this file.  
I'm only at the 10% mark and already deeply confused...

Rob
-- 
Steve Ballmer: Innovation!  Inigo Montoya: You keep using that word.
I do not think it means what you think it means.



More information about the busybox mailing list