devfs
[Top] [All Lists]

Re: [PATCH] configuration for modprobe

To: rgooch@xxxxxxxxxxxxxxx
Subject: Re: [PATCH] configuration for modprobe
From: Yann Droneaud <ydroneaud@xxxxxxxxxxx>
Date: Fri, 23 Nov 2001 13:35:15 +0100 (MET)
Cc: ydroneaud@xxxxxxxxxxx, devfs@xxxxxxxxxxx
In-reply-to: <200111230427.fAN4RO207006@xxxxxxxxxxxxxxxxxxxxxxxx> (message from Richard Gooch on Thu, 22 Nov 2001 21:27:24 -0700)
References: <200111162309.AAA05838@xxxxxxxxxxxxxxxxxxxxxxxx> <200111230427.fAN4RO207006@xxxxxxxxxxxxxxxxxxxxxxxx>
Sender: owner-devfs@xxxxxxxxxxx
> Date: Thu, 22 Nov 2001 21:27:24 -0700
> From: Richard Gooch <rgooch@xxxxxxxxxxxxxxx>
> Cc: devfs@xxxxxxxxxxx
> References: <200111162309.AAA05838@xxxxxxxxxxxxxxxxxxxxxxxx>
> Content-Length: 2392
> 
> Yann Droneaud writes:
> > here is a quite big patch against devfsd v1.3.18.
> 
> Yes, too big. It does more than one thing, so it makes it harder for
> me to pick and choose things I like and reject things I don't like.
> Since there's bits I didn't like, the whole patch is rejected.
> 

Ok, but i don't think i can split it, since all things are related.


> > This enable user to select which modprobe to use,
> > This also let the user specify arguments to pass to it.
> >  The config options could be put in devfsd.conf or on command line.
> >  Command line options have a higher priority.
> 
> I think this level of flexbility isn't needed. I have yet to see a
> good reason why modprobe isn't in /sbin. Furthermore, adding these
> configuration options bloats the code more. I'm taking a hard line
> with extra code, because I want devfsd to be very small. Think
> embedded systems.

Yes it's not designed for embedded target,
 but it could be a build time option (a #define FULL_DEVFSD)
 to make it more versatile. It's up to you. If you decided it's really
not interesting, don't include those functionnalities. 
I will use them for my private use (quite limited, i could simply hard code
 the modprobe and flags i wanted)

> 
> I'll make some comments about things I noticed in the patch, so you
> can get an idea of my likes and dislikes.
> 
> > +   Last updated by Yann Droneaud <ydroneaud@xxxxxxxxxxx> 16-NOV-2001: 
> > support for
> > +  modprobe configuration (which modprobe use and with what parameters)
> 
> Including the email address isn't the style I like: it fouls up the
> formatting.
> 
> > -#define MAX_ARGS (6 + 1)
> > +#define MAX_ARGS     (6 + 1)
> > +#define MAX_ARGV_ARGS 16 /* number of arguments passed to modprobe */
> >  #define MAX_SUBEXPR  10
> 
> Gratuitous formatting change. Not something that should be included
> with a patch that makes functional changes.
> 
> > -    char *argv[MAX_ARGS + 1];  /*  argv[0] must always be the programme  */
> > +    char *argv[MAX_ARGS + 1];  /*  argv[0] must always be the program  */
> 
> Grrr, "programme" is *correct* spelling. This is just annoying. Before
> attempting to correct my spelling, check with the OED (Oxford English
> Dictionary).

Sorry, i didn't know that correct spelling is 'programme', this sound too
much like the french word. This is why i 'corrected' them.

> 
> > +  if (modprobe.path_config[0] != '\0')
> > +    return modprobe.path_config; /* use the one found in config file */
> 
> You missed a "the" in the comment.
> 
> > +    /* can't compute args only one time, must be resetup when devfsd.conf 
> > is reread (SIGHUP) */
> 
> "resetup" isn't a real word.

Frenchies like me are not rigorous with the language. This is the only
 reasons i see ;)

> 
> > +    /* compute really dynamic parameter (those that don't depend of us 
> > directly) */
> 
> Typo? Should be "on" instead of "of"?
> 
> > -   SYSLOG (LOG_ERR, "error forking for programme: %s\t%s\n",
> > +   SYSLOG (LOG_ERR, "error forking for program: %s\t%s\n",
> 
> More bogus spelling corrections.
> 

;)

>                               Regards,
> 
>                                       Richard....
> Permanent: rgooch@xxxxxxxxxxxxx
> Current:   rgooch@xxxxxxxxxxxxxxx
> 
> 

Thank you for your comments, I will try to handle them.


-- 
Yann Droneaud      <ydroneaud@xxxxxxxxxxx>     +33 6 88 40 82 43
<ydroneaud@xxxxxxxxxxxxx> <yann.droneaud@xxxxxxxxxxxxxxxxxxxxxx>
1024D/BEA43321 5D91 B5B0 5137 B8FE 6882 FE19 CAA0 6F05 BEA4 3321
http://www.meuh.eu.org         http://www-iupmime.univ-lemans.fr

<Prev in Thread] Current Thread [Next in Thread>