Re: Is there a bug in hotplug.functions w/ ${TYPE}.usermap?

[Stamatis Mitrofanis <ewstam@softhome.net>]

David Brownell wrote:

 >"not very unreadable" = double negative = readable ;)
 >
Oops... I really hate ugly code though.

 >>It's the agent's job to map requests to drivers and modutil's job to
 >>load kernel modules. Using a load_driver function between these two is
 >>bad because it complicates things.
 >>
 >Not all drivers are kernel modules, and load_driver understands that.
 >It also invokes the driver setup scripts.
 >
Well, not all drivers are drivers, if you ask me. The net agent, for
example, doesn't "load" drivers at all. The interface with the kernel
describes "events", not directly requests for drivers.

 >>Why is the requirement for either *modules or the function *_map_modules
 >>from load_driver when the mapping can be done by the agent (possibly by
 >>calling its helper program)?
 >>
 >Uh, the agent _is_ doing the mapping, possibly by calling its helper 
program.
 >It just delegates all the common logic to a helper function, so when 
there's
 >a bug in the common logic it gets fixed once not N times.
 >
There's not much in this "common logic" anyway. The way the function is
written makes it seem like there is. It's huge and full of unnecessary
things


Shell functions are useful as "emulations" of actual programs. Defining
the meaning of *_map_modules doesn't really do much when there can be a
shell script implementation of *modules (which is already defined to do
the same job).

So off goes the *_map_functions bloat of load_driver . It can simply use
*modules .

/****/

Next is a "case" if the LISTER program doesn't exist. If the load_driver
function is called,, the *modules "command" _must_ exist. Otherwise,
what's the point?

Inside the case there are agent-specific parts. This is the worst place
to find agent-specific code. This is a "global function", a "shared
logic", so put that code in the _agent scripts_!

So off goes that bloat from load_driver.

/****/

What is left is the ugly-looking part. An amalgam of ifs, elifs, finds
etc. that I still can't read, no matter how much I look at them. Here's
the actual logic:

for every module i have to load
     IF it's already loaded, forget it
     IF it's blacklisted, forget it
     TRY to load it
     IF it's not loaded ok, tell user and forget it
     TRY to run a module-specific setup script
next module

The "already loaded" part is the job of modprobe, so it's unnecessary.


/****/

So, what's left? Three things.

One "for" loop for every module.
Two driver-specific treatments: the blacklist and the setup script.

The setup script is not flexible because it is called only _after_ the
module is loaded, allowing no driver-specific pre-installation.

So, here's my idea:

instead of calling the script _after_ the module is loaded...

_call_the_script_instead_of_loading_the_module_ ,
and if it doesnt exist, do load the kernel module.

This also gets rid of the need for a "blacklist". There can simply be
empty scripts!

/** Bloat removed. **/

So, now, what's left? A "for" and an "if". Really buggy.

I hope you got my point. (whew) The load_module function is not much of
an abstraction anyway.



_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel