[RFC, PATCH] passwd size reduction

[Bernhard Fischer]

On Thu, Nov 30, 2006 at 02:23:16AM +0100, Denis Vlasenko wrote:
>On Monday 27 November 2006 19:00, Bernhard Fischer wrote:
>> Hi,
>> 
>> I'm attaching a (minor, non-intrusive) shrinkage of passwd. Note that
>> it was only compile-tested, so don't check this in as-is!
>> 
>> Tito, vda, i'd be glad if you'd give it a whirl, if you find the time.
>> You need to apply all patches in ascending order.
>> 
>> thanks!
>> 
>> passwd.o.orig:	current svn
>> passwd.o.01b:	remove get_algo. the helptext needs updating to remove
>> sha1 since this was not supported anyway, AFAICS.
>> passwd.o.01c:	move some _main state vars into 'opt'; includes
>> non-existing 01b-patch, sorry.
>> passwd.o.01d:	move some more _main uid-related state vars into 'opt'
>> passwd.o.01e:	sanitize error pathes
>> passwd.o.01f:	sanitize some more error pathes and reuse more of libbb
>> 
>> $ size passwd.o*
>>    text    data     bss     dec     hex filename
>>    2984       0     135    3119     c2f passwd.o.orig
>>    2984       0     135    3119     c2f passwd.o.01b
>>    2895       0     135    3030     bd6 passwd.o.01c
>>    2887       0     135    3022     bce passwd.o.01d
>>    2824       0     135    2959     b8f passwd.o.01e
>>    2810       0     135    2945     b81 passwd.o.01f
>
>Last patch look strange. I am not anti-goto zealot,
>but it looks like there's a bit too many of those.

Well, it did save size.
>
>        if (access(bb_path_shadow_file, F_OK) == 0) {
>-               snprintf(filename, sizeof filename, "%s", bb_path_shadow_file);
>+               filename = concat_path_file(NULL, bb_path_shadow_file);
>        } else
>
>Isn't it just a contrived way to do xstrdup?

You are right, xstrdup is what the code wants there.
>
>I applied the rest. Also started to look into passwd.c... well...
>not very pretty.

it's ugly, yes. Tito rewrote it a while back, but the patch was not
applied, IIRC. Not sure about the reasoning.

http://busybox.net/lists/busybox/2006-November/025406.html

I didn't have a chance to look at Tito's reimpl, but if it's smaller
than what we have now (including the goto-ification), then by all
means, apply it. It can't be worse than the current incarnation ;)

thanks for the review and cheers,
Bernhard