From: David Kastrup <dak@gnu.org>
To: John Keeping <john@keeping.me.uk>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 1/5] notes-utils: handle boolean notes.rewritemode correctly
Date: Sun, 16 Feb 2014 17:22:45 +0100 [thread overview]
Message-ID: <87txbzvxgq.fsf@fencepost.gnu.org> (raw)
In-Reply-To: <be9b384ec77fc39b939b8c5505862a6e1c641faa.1392565571.git.john@keeping.me.uk> (John Keeping's message of "Sun, 16 Feb 2014 16:06:02 +0000")
John Keeping <john@keeping.me.uk> writes:
> If we carry on after outputting config_error_nonbool then we're
> guaranteed to dereference a null pointer.
Not really relevant to this patch, but looking at the output of
git grep config_error_nonbool
seems like a serious amount of ridiculousness going on. The header
shows
cache.h:extern int config_error_nonbool(const char *);
cache.h:#define config_error_nonbool(s) (config_error_nonbool(s), -1)
and the implementation
config.c:#undef config_error_nonbool
config.c:int config_error_nonbool(const char *var)
Presumably this was done so that the uses of config_error_nonbool can be
recognized as returning -1 unconditionally.
But is that worth the obfuscation? Why not let config_error_nonbool
return -1 in the first place? It does not appear like any caller would
call the function rather than the macro, so why declare the function as
returning an int at all? And why give it the same name as the macro
(risking human/computer confusion and requiring an explicit #undef for
the definition or was that declaration?) instead of
config_error_nonbool_internal or whatever else?
--
David Kastrup
next prev parent reply other threads:[~2014-02-16 16:22 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-16 16:06 [PATCH 0/5] Miscellaneous fixes from static analysis John Keeping
2014-02-16 16:06 ` [PATCH 1/5] notes-utils: handle boolean notes.rewritemode correctly John Keeping
2014-02-16 16:22 ` David Kastrup [this message]
2014-02-18 7:46 ` Jeff King
2014-02-18 8:41 ` David Kastrup
2014-02-18 9:01 ` Jeff King
2014-02-18 9:36 ` David Kastrup
2014-02-16 16:06 ` [PATCH 2/5] utf8: fix iconv error detection John Keeping
2014-02-16 16:06 ` [PATCH 3/5] utf8: use correct type for values in interval table John Keeping
2014-02-16 16:06 ` [PATCH 4/5] builtin/mv: don't use memory after free John Keeping
2014-02-16 16:06 ` [PATCH 5/5] streaming: simplify attaching a filter John Keeping
2014-02-18 23:56 ` Junio C Hamano
2014-02-19 0:02 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87txbzvxgq.fsf@fencepost.gnu.org \
--to=dak@gnu.org \
--cc=git@vger.kernel.org \
--cc=john@keeping.me.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.