On Tue, Nov 22, 2011, Eitan Adler wrote:
> The problem with profiling this type of change is that it is hard to
> find a good representative benchmark. I could easily write code that
> will show you that adding the equality check is a good idea or that it
> is a horrible idea. IMHO it saves enough time when they are equal, but
> loses almost no time when the strings are not equal.
Benchmark or not, I think you'll have a very hard time finding a
single real program that routinely calls strcasecmp() with
> Here is what I'd like to do next:
> - fix bde@'s style nits
> - change the | to a || and remove the comment
> - but leave the equality check as is.
> - find a src committer to approve the patch
> - go back to working on ports for a while ;)
> Is this the right course of action? Or should I just revert both
> commits entirely?
Fixing the style nits would be good, and it sounds like several
people are in favor of using boolean operators to combine boolean
expressions. I don't think the extra tests you added make much
difference either way, but I wouldn't want to see even more
special cases sprinkled throughout the source tree without some
validation that shows they are helpful.
You shouldn't let this discourage you from working on src in the
future. Not everyone is going to agree with everything you want
to change. One observation: The level of nit-picking tends to be
inversely proportional to the size of the change you're making.
In other words, if you put a lot of effort into a really
significant improvement, then it's *your* code and people are
less likely to bother you about the details.