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.
> 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.
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).
> + 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.
> + /* 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
|