On Mon, 2009-06-15 at 10:14 +0100, Alan Jenkins wrote:
> On 6/13/09, Andreas Robinson <andr345@xxxxxxxxx> wrote:
> > Here they are:
> >
> > git://github.com/andr345/module-init-tools.git modprobe_main
> >
> > The option-handling patches were merged before.
> >
> > Andreas Robinson (5)
> > modprobe: trivial code reorganization
> > modprobe: rename some option variables
>
> More naming fun :-).
I appreciate your patience. :-) And thanks for reviewing.
>
> "ignore_inuse" seems misleading. It's more like "ignore_loaded".
Fixed.
>
> > modprobe: remove broken -w option
... and I've deleted the docmentation now too. Nice catch btw. I had
completely forgotten about that.
> > modprobe: merge option flags into a single parameter
>
> > +static void rmmod(modprobe_flags_t flags,
>
> Conventionally, the "flags" argument goes at the end of the list.
Fixed.
>
> > - dry_run = 1;
> > - ignore_inuse = 1;
> > + flags |= mit_dry_run | mit_ignore_inuse;
>
> Please can has
>
> flags |= mit_dry_run;
> flags |= mit_ignore_inuse;
Fixed.
>
> > modprobe: move modprobing from main() into separate function.
>
> Apart from these comments, it looks reasonable. I do feel funny when
> I see do_insmod / rmmod taking some flags which they ignore (e.g.
> mit_remove). We could do something like
>
> #define RMMOD_FLAGS (mit_dry_run | ...)
>
> either just before the definition of do_rmmod, or as part of the enum
> definition. Then use "do_rmmod(... flags & RMMOD_FLAGS)". But that's
> just an idea, I'm not certain it's an improvement.
I was thinking along the same lines, but couldn't decide either. Let's
just leave it the way it is, unless Jon thinks otherwise.
Ok, let's try this pull-request thing:
The following changes since commit 22604fda4de92559b108cde7df298a5d0627fa0e:
Alan Jenkins (1):
modindex: simplify option handling
are available in the git repository at:
git://github.com/andr345/module-init-tools.git modprobe_main
Andreas Robinson (7):
modprobe: trivial code reorganization
modprobe: rename some option variables
modprobe: remove broken -w option
modprobe: merge option flags into a single parameter
modprobe: move modprobing from main() into separate function.
doc: delete modprobe -w option documentation
modprobe: fix various simple style issues related to the option flags
doc/modprobe.sgml | 14 -
modprobe.c | 684 ++++++++++++++++++++++++++---------------------------
2 files changed, 338 insertions(+), 360 deletions(-)
Cheers,
Andreas
>
> Thanks
> Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-modules" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
