On 2014-06-17 09.34, Jeremiah Mahler wrote:
> Add a strnncmp() function which behaves like strncmp() except it takes
> the length of both strings instead of just one.
>
> Then simplify tree-walk.c and unpack-trees.c using this new function.
> Replace all occurrences of name_compare() with strnncmp(). Remove
> name_compare(), which they both had identical copies of.
>
> Version 2 includes suggestions from Jonathan Neider [1]:
>
> - Fix the logic which caused the new strnncmp() to behave differently
> from the old version. Now it is identical to strncmp().
>
> - Improve description of strnncmp().
>
> Also, strnncmp() was switched from using memcmp() to strncmp()
> internally to make it clear that this is meant for strings, not
> general buffers.
I don't think this is a good change, for 2 reasons:
- It changes the semantics of existing code, which should be carefully
reviewed, documented and may be put into a seperate commit.
- Looking into the code for memcmp() and strncmp() in libc,
I can see that memcmp() is written in 13 lines of assembler,
(on a 386 system) with a fast
repz cmpsb %es:(%edi),%ds:(%esi)
working as the core engine.
strncmp() uses 83 lines of assembler, because after each comparison
the code needs to check of the '\0' in both strings.
- I can't see a reason to replace efficient code with less efficient code,
so moving the old function "as is" into a include file, and declare
it "static inline" could be the first step.
Having code inline may open the door for the compiler to decide,
"Oh, I know exactly what memcmp() does, so I through in a handfull
of lines assembly code, instead of calling memcmp() from libc".
And another thing:
What does cache_name_compare(name, namelen, ce->name, len))
in name-hash.c do?
Isn't that the same function ?
I like strnncmp() better than
cache_name_compare() or name_compare(),
but I agree with Erik here that strnncmp() has the potential to
become a name clash some day, so that git_strnncmp() may be better.
Thanks for the effort, cleaning up is needed.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
