[PATCH] deluser size reduction

[Tito]

On Monday 27 November 2006 02:00, you wrote:
> On Sunday 26 November 2006 18:06, Tito wrote:
> > Hi,
> > this patch reduces the size of deluser/delgroup.
> > Bloatcheck is:
> > 
> > function                                             old     new   delta
> > del_line_matching                                    265     219     -46
> > ------------------------------------------------------------------------------
> > (add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-46)             Total: -46 bytes
> > 
> > The patch is tested and seems to work fine at least for me.
> > Please apply if you like it.
> 
> +       if (!(passwd = fopen_or_warn(filename, "r"))) return;
> 
> I still don't like assignments in if()s.
> 
> +       while ((line = xmalloc_fgets(passwd))) {
> +               if (line[strlen(login)] == ':'
> +               && !strncmp(line, login, strlen(login))) {
> 
> First check can access line past the end.

Yes, you are right, don't thought about this.

> +       fclose(passwd);
> +
> +       if ((passwd = fopen_or_warn(filename, "w"))) {
> +clean_up:
> +               while ((line = llist_pop(&plist))) {
> +                       if (found) fputs(line, passwd);
> +                       free(line);
>                 }
> -               free(buffer);
> +               fclose(passwd);

Ops, slipped in from a previous version.

>         }
> 
> Well, we can skip closing/freeing (we are size maniacs, remember?)
> 
> BTW, as a user of this applet, can you help me with this:
> 
>         if (ENABLE_DELUSER && applet_name[3] == 'u') {
>                 del_line_matching(argv[1], bb_path_passwd_file);
>                 if (ENABLE_FEATURE_SHADOWPASSWDS)
>                         del_line_matching(argv[1], bb_path_shadow_file);
>         }
>         del_line_matching(argv[1], bb_path_group_file);
>         if (ENABLE_FEATURE_SHADOWPASSWDS)
>                 del_line_matching(argv[1], bb_path_gshadow_file);
> 
Denis,

        if (passwd) {
                if (ENABLE_FEATURE_CLEAN_UP) {
 clean_up:
                        while ((line = llist_pop(&plist))) {
                                if (found) fputs(line, passwd);
                                free(line);
                        }
                        fclose(passwd);
                } else {
                        /* found != 0 here, no need to check */
                        while ((line = llist_pop(&plist)))
                                fputs(line, passwd);
                }
        }

Maybe:

        if (!found) {
                bb_error_msg("can't find '%s' in '%s'", login, filename);
                goto clean_up;
        }

        if (ENABLE_FEATURE_CLEAN_UP) fclose(passwd);

        passwd = fopen_or_warn(filename, "w");
        if (passwd) {
clean_up:
                        while ((line = llist_pop(&plist))) {
                                if (found) fputs(line, passwd);
                                free(line);
                        }
                        if (ENABLE_FEATURE_CLEAN_UP) fclose(passwd);
                }
        }

I would free the list independently of ENABLE_FEATURE_CLEAN_UP.
On a system with a lot of users the function
is called (max) 4 times on each run and maybe could 
go out of memory? 
 

> so, "deluser joe" will delete "joe" from /etc/passwd AND
> from /etc/group? Why?? Maybe there should be a "return"
> (see attached)?

This is what deluser does on my system.

> Yes, I know that you didn't touch that part of the applet
> and this bug (if it is) isn't yours.
> 
> Okay, new deluser.c is committed to svn and also attached.
> Please test.

Will test it in the afternoon.

Ciao,
TIto
> Thanks!
> --
> vda
>