[PATCH] more devfsd clean up and size reduction

[Tito]

On Sunday 01 July 2007 20:07:51 Denis Vlasenko wrote:
> On Saturday 30 June 2007 14:16, Tito wrote:
> > Hi,
> > as nobody complained about my previous devfsd patch
> > (....i presume because nobody is using it ;-) )
> > here is one more round
> > of size reduction and code clean style clean up.
> > This patch removes the functions
> > msg_logger, msg_logger_and_die, fork_and_execute
> > and uses libbb stuff instead.
> > 
> > Bloat-o-meter says:
> > 
> > root at localhost:~/Desktop/busybox# scripts/bloat-o-meter busybox_old busybox_unstripped
> > function                                             old     new   delta
> > expand_variable                                      756     759      +3
> > get_variable                                         259     257      -2
> > get_uid_gid                                          122     120      -2
> > do_ioctl_and_die                                      27      25      -2
> > st_expr_expand                                       488     485      -3
> > signal_handler                                        94      91      -3
> > devfsd_main                                          842     833      -9
> > service_name                                        2444    2433     -11
> > read_config_file                                    1119    1098     -21
> > msg_logger_and_die                                    31       -     -31
> > .rodata                                           125143  125111     -32
> > msg_logger                                            88       -     -88
> > fork_and_execute                                      95       -     -95
> > ------------------------------------------------------------------------------
> > (add/remove: 0/3 grow/shrink: 1/9 up/down: 3/-299)           Total: -296 bytes
> > 
> > BE WARNED THAT THIS IS ONLY COMPILE TESTED!!!
> > 
> > If there are some volunteers for testing it .....
> > 
> > Denis could you please take a look at my use
> > of spawn, xspawn, wait4pid and bb_daemonize_or_rexec
> > as I never used them before and i'm not quite sure 
> > if i've done things right.............
> > and apply the patch if you like it.
> 
> Looks like you should indeed use fopen, not xfopen here.
> You test it for NULL, and do not seem to exit if it fails:

Hi,
i see, will fix it.

> -               if ((fp = fopen(path, "r")) != NULL) {
> +               if ((fp = xfopen(path, "r")) != NULL) {
>                         while (fgets(buf, STRING_LENGTH, fp) != NULL) {
>                                 /*  Skip whitespace  */
> -                               for (line = buf; isspace(*line); ++line)
> -                                       /*VOID*/;
> +                               line = buf;
> +                               line = skip_whitespace(line);
>                                 if (line[0] == '\0' || line[0] == '#')
>                                         continue;
>                                 process_config_line(line, event_mask);
>                         }
>                         fclose(fp);
> -               } else {
> -                       goto read_config_file_err;
>                 }
>         } else {
> -read_config_file_err:
> -       if (optional ==  0  && errno == ENOENT)
> -               msg_logger_and_die(LOG_ERR, "read config file: %s: %m", path);
> +               if (optional ==  0  && errno == ENOENT)
> +                       error_logger_and_die(LOG_ERR, "read config file: %s", path);
>         }
>  }   /*  End Function read_config_file   */
> 
> 
> Here you should pass argv, not NULL, to 2nd parameter of bb_daemonize_or_rexec:

I suspected that but was lost in the define forest  of this code...

> -               fork_and_execute(DIE, NULL, NULL);
> -               setsid();        /*  Prevent hangups and become pgrp leader         */
> +               bb_daemonize_or_rexec(0, NULL);
> 
> It basically will pass argv back to devfsd_main() in re-executed child
> (If you run on NOMMU CPU. On MMU, it simply daemonize and ignore argv).
> 
> 
> Looks right:
> 
> -       fork_and_execute(NO_DIE, argv[0], argv);
> +       wait4pid(spawn(argv));
> 
> 
> 
> 
> I like devfsd.c :)  Look at this beauty:

:-) this is my personal creation........did it when i was young...
i planned to substitute it with bb_simplify_path,
will add it to the improved patch (this saves about 40b),
don't ask me why i put it there....I'm sure there was some reason...
but I can't remember it....

>         /* strip last / from mount point, so we don't need to check for it later */
>         while (argv[1][1] != '\0' && argv[1][strlen(argv[1]) - 1] == '/')
>                 argv[1][strlen(argv[1]) - 1] = '\0';
> 
> --
> vda
> 

Ciao,
Tito
-------------- next part --------------
A non-text attachment was scrubbed...
Name: devfsd04.patch
Type: text/x-diff
Size: 13066 bytes
Desc: not available
Url : http://lists.busybox.net/pipermail/busybox/attachments/20070701/d204c480/attachment-0002.bin