Re: [PATCH 5/5 the last one, really!] modprobe, tests: make modprobe fail noisily on non-ELF files

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

 



On 5/21/09, Andreas Robinson <andr345@xxxxxxxxx> wrote:
> On Thu, 2009-05-21 at 12:33 +0100, Alan Jenkins wrote:
>> On 5/21/09, Andreas Robinson <andr345@xxxxxxxxx> wrote:
>> >
>> >  		error("Could not read '%s': %s\n",
>> >  		      mod->filename, strerror(errno));
>>
>>
>> This will change the error message to something less helpful.  You
>> should probably use insert_moderror() instead of strerror().
>
> I'm not so sure about that. strerror(ENOEXEC) == "Exec format error",
> and while I agree it is less clear than "invalid module format", it is
> exactly the right message for this kind of error. I.e we're trying to
> load an executable but it really isn't => Exec format error. It is
> simply seen less often than say, "File not found".

Yeah, I didn't say it was actually wrong.

> To use insert_moderror(), the message would have to be formatted like
> this:
>
> error("Could not read '%s': %s\n", mod->filename,
> 	(errno == ENOEXEC) ? insert_moderror(errno) : strerror(errno));
>
> Though this is probably better:
>
> error("Could not read '%s': %s\n", mod->filename,
> 	(errno == ENOEXEC) ? "invalid module format" : strerror(errno));
>
> since insert_moderror() is written explicitly for init_module().
>
> If it isn't done like that, we would get insert_moderror(ENOENT) ==
> "Unknown symbol in module, or unknown parameter (see dmesg)" which
> clearly isn't the "File not found" we're expecting.

Agreed, that would be very wrong.  The second version above looks good.
--
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

[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux