Updated mdev

Rob Landley rob at landley.net
Mon Dec 12 19:59:55 UTC 2005


On Monday 12 December 2005 10:18, Frank Sorenson wrote:

> > Looking more closely, you also seem to have reimplemented strcat
> > (string_cat()), strncmp (string_cmpn()), memset (clear_mem())...  Why did
> > you do this?
>
> Mostly because I seemed to be able to reimplement them smaller than if I
> just used the functions.
>
> "I'm an idiot" factors highly into this, I'm sure ;)

Well, gcc can definitely lead to that sort of thinking.  Linking against 
uClibc and klibc, however, tends to give much more sane results.

> > Looking at get_device_info (which is another function that's called from
> > only one place): BUFFER_LENGTH was chosen how?  (I'd be more comfortable
> > with bb_asprintf() here.)  The check if (ret ==0 ) leaks the filehandle,
> > and doesn't catch the actual error return value (-1) which would leading
> > to the buf[ret-1]=0 assignment writing outside the buffer...
> >
> > All this stuff with netlink needs a CONFIG_ and #ifdefs.  Is there
> > _really_ no better header to include than linux/netlink.h and
> > linux/types.h?
>
> Not sure.  The udev code itself went this way, and I wasn't aware of
> replacements.

I haven't looked at any remotely recent udev code, but I'll take a look and 
see what else I can come up with.

> > process_netlink_message:   Apparently, it can never get a block device
> > hotplugged over netlink, just char devices.
>
> Both char and block devices should be handled.

How?

In make_device:

        if (string_cmpn(path, "/sys/block", 10))
                type = S_IFBLK;
        else if (string_cmpn(path, "/sys/class", 10))
                type = S_IFCHR;

In process_netlink_message:

        if (string_cmpn(message, "add@/class/", 11) || string_cmpn(message, 
"add@/block/", 11)) {
                message[0] = '/';
                message[1] = 's';
                message[2] = 'y';
                message[3] = 's';
                make_device(message);

And that's always going to start with /sys/class, so char device.

Where do block device messages come in via netlink?

...
> > So when this program is run with no arguments, it does nothing?  (Not
> > even a help message?)
>
> I'm still an idiot :)

Code review.  People catch stuff in my code all the time... :)

> > Did you see any of the discussion on the busybox list about a minimal
> > config file format to specify ownership and permissions for devices?
>
> Yes.  It shouldn't be too difficult to add the code to read the config
> and set permissions and ownership.

Cool.

I can code this up if you like.  I _think_ all we need is the device 
permissions and ownership syntax (with regex support), and the shellout 
syntax.  All the stuff about creating symlinks and directories we really 
don't need to do.  If we don't mount /dev or /sys then the script that does 
that (and then calls us) can do any other setup required.

> Nice.  I figured it could be fixed for busybox by someone who knew what
> they were doing.

That would be me. :)

Hmmm...  I really _shouldn't_ take a stab at this today, but considering I'm 
the one setting the schedule for all this in the first place... :)

> > On the whole it's cool, but not something I'd merge right now.
>
> Understood.  This was more a proof-of-concept and a first crack at
> implementing it small.  It's functional code, but definitely not clean
> and complete.

It's not that far from being turned into something clean and complete, though.  
I'll probably take a whack at it out of sheer momentum at this point.  (I 
always find _reading_ code to be the hard part.  Writing it is easy...)

> > I have plans to tackle mdev for busybox at the start of 1.2.  (Including
> > the config file  functionality and adding it to applets.h and usage.h and
> > kconfig all that.)  I see several chunks of code I can use in here, but
> > I'd clean it up noticeably.
>
> Please do.  It's working code, but obviously needs help.  Feel free to
> take whatever you find helpful in your rewrite.

Ooh, temptation...

Rob
-- 
Steve Ballmer: Innovation!  Inigo Montoya: You keep using that word.
I do not think it means what you think it means.



More information about the busybox mailing list