Re: [PATCH] a few small mconf improvements

[Roman Zippel <zippel@linux-m68k.org>]

Hi,

On Sat, 13 May 2006, Jesper Juhl wrote:

(Sorry for the delay.)

> > >  - if the sscanf() call in conf() fails and stat==0 && type=='t', then
> > >    we'll end up dereferencing a NULL 'sym' in sym_is_choice(). The patch
> > >    adds a NULL check of 'sym' to that path and bails out with a big fat
> > >    error message if that should ever happen (better than just crashing
> > >    IMHO).
> > 
> > That error message is as useful to the normal user as a segfault - mconf
> > doesn't work. Since it shouldn't happen, this check adds no real value,
> > the user still has to provide enough information to reproduce the problem
> > and at this point it makes no difference, whether I get this message or I
> > see where it stops with gdb.
> > 
> I disagree a little here. It may not really matter to you if you get a
> report of a crash or if you get a report that mconf spewed an error
> message, but to the user who experiences it (should it ever happen)
> there's a difference - it's either "the damn thing crashed on me, what
> a piece of crap" or "the damn thing crashed on me, but at least it
> told me something went wrong, so now I can report it"... Printing an
> error and exiting cleanly is IMHO always preferable to a crash - users
> respond better to that and it's the "right" thing to do.

The point is that this shouldn't happen and so far didn't, you have to do 
something really weird to trigger this, so a big error message is not 
appropriate. Something like assert() would be more acceptable, but IMO 
it's just not important enough.

> What about the other bits of the patch? are those OK?

Yes, it's OK.

bye, Roman
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/