On 6/13/09, Andreas Robinson <[email protected]> wrote:
> On Sat, 2009-06-13 at 10:59 +0200, Andreas Robinson wrote:
>> On Fri, 2009-06-12 at 14:57 -0400, Jon Masters wrote:
>> > On Mon, 2009-06-01 at 14:43 +0100, Alan Jenkins wrote:
> [...]
>> > > Sounds good to me. Maybe it's a little verbose, but we can only spend
>> > >
>> > > so much time debating names :-).
>
> It almost turned into a bike-shed discussion there, didn't it? :)
>
>> >
>> > Let's go with something like that. Andreas - do you have patches for
>> > this that I have not received yet, or just the idea so far?
>>
>> Just the idea so far, I haven't had any time to work on it for the past
>> week. :-p
>>
>> I have a few groundwork patches that moves the actual modprobing from
>> main() to a new function called do_modprobe(), that could replace the
>> system() calls:
>>
>> typedef enum
>> {
>> mit_remove = 1,
>> mit_dry_run = 2,
>> mit_first_time = 4,
>> mit_use_blacklist = 8,
>> mit_ignore_commands = 16,
>> mit_ignore_inuse = 32,
>> mit_strip_vermagic = 64,
>> mit_strip_modversion = 128
>>
>> } modprobe_flags_t;
>>
>> int do_modprobe(modprobe_flags_t flags,
>> errfn_t error,
>> char *modname,
>> char *newname,
>> char *cmdline_opts,
>> const char *configname,
>> const char *dirname,
>> const char *aliasfilename,
>> const char *symfilename);
>>
>> I'll adapt the patches to Alan's work and post them later today.
>>
>
> 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 :-).
"ignore_inuse" seems misleading. It's more like "ignore_loaded".
> modprobe: remove broken -w option
> 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.
> - dry_run = 1;
> - ignore_inuse = 1;
> + flags |= mit_dry_run | mit_ignore_inuse;
Please can has
flags |= mit_dry_run;
flags |= mit_ignore_inuse;
> 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.
Thanks
Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-modules" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
