[Toybox] [PATCH] Re: modinfo...

[Isaac]

On Sun, Jun 23, 2013 at 02:39:14PM -0500, Rob Landley wrote:
> On 06/20/2013 11:24:48 PM, Isaac wrote:
> >On Wed, Jun 19, 2013 at 11:44:08PM -0500, Rob Landley wrote:
> >> On 06/18/2013 08:55:42 AM, Isaac wrote:
> >> >
> >> >Oh, there's a trivial option to add to modinfo: -b <basedir>
> >> >(used for constructing an initrd-it locates the modules within
> >> >a given directory).
> >>
> >> Um, where's this from? My host's modinfo hasn't got it, and I don't
> >> know of an actual standard for this...
> >>
> >
> >I noticed it on my Lucid system:
> 
> Ok.
> 
> >So I don't see a high priority; on the other hand, it might
> >hypothetically be
> >useful if firmware requirements change in a new kernel version.
> >In other words, sounds like something to hold off on.
> 
> I haven't got an objection if it's actually being used somewhere, I
> just didn't see it on my system.

Looking at the kmod documentation, I see that the function is a little 
different from what I thought:
it's changing the root for your search (eg, -b ~/projects/out/ where that's the contents of a future boot filesystem). 
Of course, that means the patch to add -b is broken.

> >>
> >> The documentation describes this:
> >> http://landley.net/toybox/code.html#lib_args
> >>
> >> >modinfo also should support specifying an absolute or relative path
> >> >to a module; the one complication is that modinfo_file() takes a
> >> >struct dirtree, not a filename; can dirtree_read handle a filename
> >> >or a relative path?
> >>
> >> Yes: http://landley.net/toybox/code.html#lib_dirtree
> >
> >Thanks for the answer; I missed that, probably due to trying to read
> >rather late.
> 
> I feel I should make the documentation more clear, but dunno how.

You can't clarify everything. It seems obvious reading it, but try reading 
anything when you can barely stay awake...

> Wouldn't the presence of a / character indicate use of a path?
> 
> >While I am not aware of any rules forbidding a module like
> >"barf.ko.ko",
> >an strace of module-init-tools modinfo indicates that they use
> >similar logic.
> 
> Hmmm... I guess an actual file is going to have the .ko extension,
> although strstr() doesn't confirm it _ends_ with .ko... 

> The difference between if (strchr(str, '/')) and checking for .ko is
> that "modinfo filename.ko" would work with the second but not the
> first. We could also always check for the argument as a file, but
> that would mean if you do have a file in the current directory of
> the same name it would load that file, and that's a bit like not
> having "." in the $PATH by default...
> 
> Eh, changing behavior based on extension is normalish. I'd like to
> add a check to make sure it's actually at the end until the further
> extension support is implemented.

OK.
modinfo ath_pci.ko
is something that I have done from time to time.

> >It adds 6 lines, with 41 bytes increase in binary size (GCC 4.4,
> >glibc/dynamic and musl/static, according to make baseline/bloatcheck).
> >I'm not sure if the void->int change was needed, though it does
> >keep GCC
> >quiet. But perror_exit; -> perror_msg; return; was needed.
> 
> Except it's still xopen(), so it'll abort if unable to open the
> file, but return error if it can't map it. (And leak the
> filehandle.) Hmmm... all modinfo_file() actually uses is the
> filename, no reason to go through dirtree() for that... Nothing is
> actually _checking_ the return value of modinfo_file()... Huh, and
> this thing is doing a blob of global data (I try to combine globals
> into the union)...
> 
> Checking in a cleanup on top of your commit. (And I need to dig up
> the -b patch...)

Thanks.

See above (-b patch is broken).  But it's after midnight here, so I'm
not rewriting it atm.

> Thanks,
> 
> Rob