On Tuesday 19 October 2010 21:28:50 Dan Poirier wrote: > On 2010-10-19 at 15:21, "Roy T. Fielding" <[email protected]> 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
