mount -a remounts tmpfs entries: bug or feature?

[Denys Vlasenko]

On Tuesday 25 November 2008 13:30, Rob Landley wrote:
> > Other that NFS code - what places you don't like Rob?
> 
> An #ifdef for _dietlibc_,

If someone sends me a patch for this, I conclude
they do build it against dietlibc. Why not help them?

> special casing rootfs, special casing shared subtree  
> flags,


> the mount_option_str separately quoting "\0" (small but ugly, you  
> wouldn't do it for \n)...

fixed

> Some of it might just be that I've gotten used to looking at the toybox 
> infrastructure for things like option parsing, so constructs like:
> 
> #if ENABLE_BLAH
> #define ifBlah (logic)
> #else
> #define ifBlah 0
> #endif
> 
> Just look really wrong, although that one was probably my fault once upon a 
> time.

> > Lots of #if ENABLE in general that could be if (ENABLE) instead.   
> ENABLE_MOUNT_LABEL in resolve_mount_spec(), for example.

fixed resolve_mount_spec().

> In general a static  
> function should be inlineable and optimizable away with gcc 4.x.

should be != is. gcc is still sometimes rather stupid.
#if FOO is a surefire way to DEFINITELY exclude some code.
if (FOO) (actually some more complex constructs with &&, ||,
etc) confuse gcc.

> I'm looking  
> at verbose_mount() here, which only has two callers anyway so guarding it at 
> the call sites might be better, although the second call site has comments on 
> each _argument_, which is sick...
> 
> Speaking of verbose_mount(), in its second caller you have this:
> 
>         rc = verbose_mount(/*source:*/ "", /*target:*/ argv[0],
>                 /*type:*/ "", /*flags:*/ i, /*data:*/ "");

fixed

> A quick glance at the code shows it's got lots of:
>         // WARNING. I am not sure this matches util-linux's
>         // behavior. It's possible util-linux does not
>         // take -o opts from mtab (takes only mount source).
> 
> I actually explicitly tested that sort of thing over a period of 3 months and 
> worked out what the correct behavior should _be_, and implemented it at the 
> time.

Yeah, users reported discrepancy but not really followed it through.
I think I need to document in a comment as a minimum
for the benefit of whoever will hack on it further.

--
vda