Re: [PATCH] a few small mconf improvements

[Jesper Juhl]

On 09/05/06, Roman Zippel <zippel@xxxxxxxxxxxxxx> wrote:
> On Sun, 7 May 2006, Jesper Juhl wrote:
>
> >  - rename main() arguments from "ac"/"av" to the more common "argc"/"argv".
>
> conf.c and qconf.cc do the same, it's a personal preference.
Fair enough.


> >  - when unlinking lxdialog.scrltmp, the return value of unlink() is not
> >    checked. The patch adds a check of the return value and bails out if
> >    unlink() fails for any reason other than ENOENT.
>
> The check is not needed, the worst that can happen is a misbehaving
> lxdialog and you certainly have bigger problems than this, if the unlink
> should fail. In the long term this should go away anyway.
Ok, your call.


> >  - 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.

What about the other bits of the patch? are those OK?
Want me to send an updated patch - or?


--
Jesper Juhl <jesper.juhl@xxxxxxxxx>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please      http://www.expita.com/nomime.html
-
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/