[PATCH] more devfsd clean up and size reduction

[Denis Vlasenko]

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:

-               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:

-               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:

        /* 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