svn commit: trunk/busybox/util-linux

[Denis Vlasenko]

On Wednesday 20 September 2006 19:25, Rob Landley wrote:
> > so I wouldn't replace them by enums en masse. However I still think
> > that they are unsafe. I will use enums in new code if you
> > don't mind, ok?
> 
> I'm kind of attached to mount.c.  I rewrote the sucker three times.  I suspect 
> your new changes have broken it.  (Did you run the regression test script?  
> Which isn't close to complete, but it's a start.)

/me looking into it...
Will do. I need to download and learn uml.

> If you just want me to keep my hands off of mount.c in perpetuity and forward 
> all maintenance requests to it to you, perhaps I can do that.

No, it's okay to complain.

I think that review is always good for code quality, even
if sometimes reviewers get a bit hot with emotions.

For example, I already spotted an unrelated bug while looking
into your bloatcheck results.

> Except that when I did this:
> 
> svn update 16000
> make distclean
> make allnoconfig
> make menuconfig: switch on mount, mtab support, an loopback.  (Leave nfs off.)
> make baseline
> svn update
> make oldconfig  # for NFS_CIFS, which is switched off by default.
> make
> 
> I got:
> 
> function                                             old     new   delta
> bb_getopt_ulflags                                      -     884    +884
> append_mount_options                                  55     183    +128
> bb_verror_msg                                         56     130     +74
> mount_it_now                                         277     330     +53
> llist_add_to                                           -      28     +28
> bb_perror_msg_and_die                                 29      47     +18
> bb_error_msg_and_die                                  42      50      +8
> msg_eol                                                -       4      +4
> logmode                                                -       4      +4
> die_sleep                                              -       4      +4
> bb_opt_complementally                                  -       4      +4
> bb_applet_long_options                                 -       4      +4
> xstrdup                                               33      32      -1
> xrealloc                                              38      37      -1
> xmalloc                                               33      32      -1
> busybox_main                                         196     191      -5
> singlemount                                          633     626      -7
> bb_path_mtab_file                                     10       -     -10
> bb_error_msg                                          35      24     -11
> mount_main                                           970     931     -39
> bb_vperror_msg                                        74      30     -44
> .rodata                                             1104    1053     -51
> ------------------------------------------------------------------------------
> (add/remove: 7/1 grow/shrink: 5/9 up/down: 1213/-170)        Total: 1043 bytes

Almost all of 1043 bytes come from getopt -> bb_getopt_ulflags conversion
and append_mount_options fix.

Old mount was using getopt, which lives in libc, and therefore isn't visible
in bloatcheck. modified mount uses our internal one (bb_getopt_ulflags),
which lets us have far simpler code in the caller. But of course this
sucks in bb_getopt_ulflags...

I have a bunch of similar conversions for other applets which reduces bbox
size by more than 1 kb.

Fix to append_mount_options makes it so that it doesn't add option
more than once. It is required to avoid, for example,
"mount --bind" producing option string of "bind,bind".
I am sure it's easy to produce other similar double options.

mount_it_now grew because old mount had useMtab always 0, even with
mtab support selected, and entire if() was optimized away. This was fixed.
--
vda