Re: [PATCH] pstore: gracefully handle NULL pstore_info functions

[Tony Luck]

On Thu, Nov 17, 2011 at 10:42 AM, Kees Cook <keescook@xxxxxxxxxxxx> wrote:
> -       kmsg_dump_register(&pstore_dumper);
> +       if (psi->write && psi->buf && psi->bufsize)
> +               kmsg_dump_register(&pstore_dumper);

I can see that you might not need the other parts of the pstore
interface if your back-end is very simple (e.g. only has space for a
single record). But stub functions are cheap - so it isn't clear who
should pay the overhead.

BUT - without psi->write!? What use is it!?  I think in an early version
I refused the registration with -EINVAL - but akpm convinced me it
was a waste of electrons:

On Wed, 1 Dec 2010 16:51:39 Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
>> +     if (!psi->reader || !psi->writer || !psi->eraser) {
>> +             spin_unlock(&pstore_lock);
>> +             return -EINVAL;
>
> It doesn't seem appropriate to check this here.  It's a programming
> error!  Just install the thing and let the kernel oops - the programmer
> will notice.

Hiding the lack of a ->write function by quietly accepting the registration, but
not hooking into kmsg_dump doesn't sound useful in any scenario.

-Tony

Dang - pstore is coming up on a year old - time flies when you're having fun.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/