Mustafa Orkun Acar <mustafaorkunacar@xxxxxxxxx> writes:
> I reviewed all functions using memcmp(). It generally makes code more understandable. But here it might be used for the sake of simplicity.
>
> Signed-off-by: Mustafa Orkun Acar <mustafaorkunacar@xxxxxxxxx>
> ---
> I applied to GSoC 2014. I expect your feedbacks and comments!
> strbuf.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/strbuf.c b/strbuf.c
> index ee96dcf..50d0875 100644
> --- a/strbuf.c
> +++ b/strbuf.c
> @@ -147,7 +147,7 @@ void strbuf_list_free(struct strbuf **sbs)
> int strbuf_cmp(const struct strbuf *a, const struct strbuf *b)
> {
> int len = a->len < b->len ? a->len: b->len;
> - int cmp = memcmp(a->buf, b->buf, len);
> + int cmp = !starts_with(a->buf, b->buf);
> if (cmp)
> return cmp;
> return a->len < b->len ? -1: a->len != b->len;
Not correct. The original code clearly takes care to return a signed
result with the same definition of signedness as memcmp. While this
intent has not been written down in a comment or description in either
strbuf.c or strbuf.h, the code does not make sense without it.
rerere.c contains the following lines:
if (strbuf_cmp(&one, &two) > 0)
strbuf_swap(&one, &two);
and that only makes sense when there is an actual meaning to the sign of
the result.
Your version would return 1 when either comparing "1" with "2" OR "2"
with "1". It requires NUL-terminated strings: if that was a valid
constraint for strbuf, this function would be using strcmp in the first
place.
--
David Kastrup
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html