Re: depends command

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]


On 6/13/09, Andreas Robinson <andr345@xxxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Home]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]    [Yosemite Photos]    [Video Projectors]     [PDAs]     [Free Online Dating]     [Hacking TiVo]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Devices]     [Big List of Linux Books]     [16.7MP]

Add to Google Powered by Linux