Messages in this thread | | | Date | Sat, 13 May 2006 22:59:21 +0200 | From | "Jesper Juhl" <> | Subject | Re: [PATCH] a few small mconf improvements |
| |
On 09/05/06, Roman Zippel <zippel@linux-m68k.org> 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@gmail.com> 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |