Subject: Re: [PATCH 1/5] notes-utils: handle boolean notes.rewritemode correctly

From: David Kastrup <[email-masked]>
Date: Sun, 16 Feb 2014 17:22:45 +0100

John Keeping <[email-masked]> 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