httpd-dev mailing list archives

Re: [PATCH] How to Use strcmp to Check for Equality Without Confusing Your Fellow Code, Or: Isn't There a Macro for That?

Date

Wed, 20 Oct 2010 11:58:07 GMT

On Tuesday 19 October 2010 21:28:50 Dan Poirier wrote:
> On 2010-10-19 at 15:21, "Roy T. Fielding" <fielding@gbiv.com> wrote:
> > On Oct 19, 2010, at 9:36 AM, Malte S. Stretz wrote:
> >> And there are a lot of string compares in the Apache codebase.
> >> Everytime you see a strcmp, you (or is it only me?) have to stop
> >> and think "well, is this branch checking for equality or the
> >> opposite?"
> >>
> >> I think this is a case where either a coding standard could help, or
> >> some helper macros in APR. I went for the latter and defined
> >> APR_EQ plus variants in apr_string.h. See attached patch.
> >
> > Maybe a standard would help. More macros would not -- that pain
> > would be far worse than the current inconsistency. -1 (design
> > opinion, not veto)
>
> I tend to agree with that. Writing our code in C in a consistent way
> that will become very familiar to Apache developers and is easy to
> interpret by anyone else who's already familiar with C is better than
> trying to use macros to hide it, and thus making it unfamiliar to
> everyone.
The variants I found in the current codebase included, sometimes even
mixed in a single module:
For equality:
strcmp(...) == 0
!strcmp(...)
0 == strcmp(...)
strEQ(...)
For inequality:
strcmp(...) != 0
strcmp(...)
!(strcmp(...) == 0)
0 != strcmp(...)
strNE(...)
> Is there a commonly accepted usual way of writing this that we
> could adopt without getting into a long discussion of the relative
> merits of every possibility?
Unfortunately people are arguing about that since the C standard forgot to
include a streq() function :)
While I personally don't like the second variant because the logic is
inverted, it is a common idiom and easy to understand as long as everybody
uses the same way to write this. I guess Michael's crib, seeing strcmp as
a function which checks for inequality (which is essentially also the
meaning of the compare-to-zero) would be a way justify it.
Just the current mix-and-match approach makes scanning the code harder
than it should be. So some guideline in [1] would be nice.
Cheers,
Malte
[1]http://httpd.apache.org/dev/styleguide.html