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