svn commit: trunk/busybox/util-linux

[Rob Landley]

On Thursday 21 September 2006 7:53 am, Denis Vlasenko wrote:
> 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.

I have a todo item to make it work with qemu.  I can bang on that in the next 
evening or so if you like.

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

Yeah, I guess that's not so bad.  I need to get back to scripts/individual and 
teach it to use the shared library.  (Need to figure out how I want to 
specify that.  At some point in the future, I'll probably tie it back into 
the rest of the build infrastructure (either call it from the build 
infrastructure or rewrite it), but that's _after_ the switch to kbuild.  
(How's that going, by the way?  Should I go poke at the previous drop, or are 
you likely to do anything with the klibc stuff the kbuild maintainer pointed 
you at?)

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

It's a busybox app, we'll live. :)
 
> I have a bunch of similar conversions for other applets which reduces bbox
> size by more than 1 kb.

Cool.

I've been holding off on doing anything with that because I've got a partial 
rewrite of bb_getopt_ulflags sitting abandoned in one of my subdirectories.

One of the reasons I'm so interested in mercurial is it lets me do this kind 
of side project without the copy of the tree it's in getting so stale that 
working on it becomes a big production because the first thing I have to do 
is port it to a current tree.  Unfortunately, "svn update" craps >>>>>mine 
stanzas all over the thing, which annoys me to no end...

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

So you're willing to guarantee that no filesystem out there wants to be fed 
the same filesystem-specific option more than once, ala the "ssh -v -v -v" 
construct?

Where was this getting added?  Why was this getting added?  My code never 
called append_mount_options() with an option that was already in the string.  
I didn't add a band-aid to catch unnecessary calls, I didn't _make_ 
unnecessary calls.  For that reason, I don't think append_mount_options() is 
the right place to fix it.

The caller can just go:

if (!(MS_BIND & parse_mount_options(options,NULL)))
	append_mount_options(options,"bind");

This is a lot smaller than adding 19 lines to append_mount_options().  
However, _why_ is this needed?  What's making the call again?

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

Yeah, I remember that.  I plan to go look at it more closely once it stops 
moving around.  I remember testing some funky corner cases with legacy mtab 
support, but apparently I missed checking something in.

Rob
-- 
Never bet against the cheap plastic solution.