Re: [PATCH] a few small mconf improvements

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/

Relevant Pages

  • Re: unspecified spotential security flaw
    ... I have uninstalled the patch and the error message went away. ... Microsoft Product Support Services at 1-866-PCSAFETY. ... charge for support calls that are associated with security updates. ...
    (microsoft.public.windows.inetexplorer.ie6.browser)
  • Re: If you cant tell me how to update MSO 2003, can you tell me..
    ... Patch could not be applied is all I've ... Please, TaurArian, at this point, all I want to do is remove the update ... Error message when you try to install some of the Office 2003 or Office XP ...
    (microsoft.public.officeupdate)
  • Re: AUTHENTICATION TO APPLY SP3
    ... When I apply the patch it asks me to choose ... I only have one instance of SQL ... >error message at all, ... >> These accounts and passwords are good because I can get ...
    (microsoft.public.sqlserver.security)
  • question about kill PIDTYPE_TGID patch
    ... the error message below: ... this is because of the PIDTYPE_TGID patch. ... unsigned long pidAddr, headAddr, nextAddr; ...
    (Linux-Kernel)
  • Re: share/private/slave a subtree - define vs enum
    ... > defines and it works fine, I don't really see a good enough reason to ... it is not as if I get to decide for the patch submitters. ... As I disagree with the part about enums being a personal preference, ... send the line "unsubscribe linux-kernel" in ...
    (Linux-Kernel)