Bug triage.

Rob Landley rob at landley.net
Mon Dec 12 19:39:11 UTC 2005


On Monday 12 December 2005 05:20, Yann E. MORIN wrote:
> Hello All!
> Rob,
>
> On Saturday 10 December 2005 005, Yann E. MORIN wrote:
> > > 276  Linux 2.6 module autoloading breaks when support for 2.4 modules
> > > is disabled
> > > 272  modprobe does not process parameters
> >
> > --> Patch attached.
> >     Heavy.
> > The attached patch about modprobe rework solves these two issues. Note
> > that it still relies on my MODPROBE_MULTIPLE_OPTIONS. Rediffed against
> > rev 12789. => busybox-modprobe.patch.bz2
>
> Attached a cleaned-up patch for modprobe.

Coolness.

                p = strchr (buffer, ' ');
                if (p) {
                        *p = 0;
+                       for( p = buffer; ENABLE_FEATURE_2_6_MODULES && *p; 
p++ )
 {
+                               *p = ((*p)=='-')?'_':*p;
+                       }


I'm not 100% certain that can cleanly be optimized away, since p lives outside 
your current scope and the for(;;) will assign to p before terminating (and 
hopefully optimizing out the body of) the loop.

One reason I've been holding on the CONFIG_->ENABLE_ conversion is to get more 
familiar with the optimization capabilities of gcc.  (What kind of guards can 
we get away with doing without accidentally bloating the code?  if(ENABLE) 
seems pretty safe.  I'd want to check this vs putting an explicit if(ENABLE) 
around it.  Might be clearer from a code reading point anyway...)

-#ifdef CONFIG_FEATURE_MODPROBE_MULTIPLE_OPTIONS
-#ifdef CONFIG_FEATURE_CLEAN_UP
-                               argc_malloc = argc;
-#endif
+                               if( ENABLE_FEATURE_CLEAN_UP )
+                                       argc_malloc = argc;

You dropped a MODPROBE_MULTIPLE_OPTIONS guard here.  I'm assuming it's 
intentional, could you walk me through the logic?  (What lets this optimize 
out, or why shouldn't it?)

-#if defined(CONFIG_FEATURE_2_6_MODULES)
-               if ( argc ) {
+               if( argc ) {

Why was that guard unnecessary?

I just checked it in.  Does it fix bugs 276 and 272?

Rob
-- 
Steve Ballmer: Innovation!  Inigo Montoya: You keep using that word.
I do not think it means what you think it means.



More information about the busybox mailing list