Mustafa Orkun Acar <mustafaorkuna...@gmail.com> 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 <mustafaorkuna...@gmail.com>
> ---
> 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to