Re: [PATCH] a few small mconf improvements

From: Jesper Juhl
Date: Sat May 13 2006 - 16:58:49 EST


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/