All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeremiah Mahler <jmmahler@gmail.com>
Cc: Jonathan Nieder <jrnieder@gmail.com>, git@vger.kernel.org
Subject: Re: [PATCH v2 1/3] add strnncmp() function
Date: Tue, 17 Jun 2014 10:55:18 -0700	[thread overview]
Message-ID: <xmqqd2e7mneh.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <50de63f47ded2337adcd8bce151190fb99b38d64.1402990051.git.jmmahler@gmail.com> (Jeremiah Mahler's message of "Tue, 17 Jun 2014 00:34:37 -0700")

Jeremiah Mahler <jmmahler@gmail.com> writes:

> Add a strnncmp() function which behaves like strncmp() except it takes
> the length of both strings instead of just one.  It behaves the same as
> strncmp() up to the minimum common length between the strings.  When the
> strings are identical up to this minimum common length, the length
> difference is returned.
>
> Signed-off-by: Jeremiah Mahler <jmmahler@gmail.com>
> ---
>  strbuf.c | 9 +++++++++
>  strbuf.h | 2 ++
>  2 files changed, 11 insertions(+)
>
> diff --git a/strbuf.c b/strbuf.c
> index ac62982..4eb7954 100644
> --- a/strbuf.c
> +++ b/strbuf.c
> @@ -600,3 +600,12 @@ char *xstrdup_tolower(const char *string)
>  	result[i] = '\0';
>  	return result;
>  }
> +
> +int strnncmp(const char *a, int len_a, const char *b, int len_b)
> +{
> +	int min_len = (len_a < len_b) ? len_a : len_b;
> +	int cmp = strncmp(a, b, min_len);
> +	if (cmp)
> +		return cmp;
> +	return (len_a - len_b);
> +}

I am not sure if the interface into this function conceptually makes
much sense.  strncmp(entry, string, 14) was invented as the way to
see if a NUL terminated "string" matches with the contents in an
array of char "entry" that is up to 14 bytes long, and because the
"entry" was allowed to fill full 14-byte space without terminated
with a NUL, the maximum possible length is specified separately, but
a NUL termination in "entry", if exists, is still honored.  Is there
any case where such a pair of "maximum N bytes but could be shorter"
strings are compared, especially with different N's defined per
string, in our codebase (or in other people's project for that
matter)?

Further, I do think that the interface into this function and its
implementation are inappropriate for implementing the name_compare()
function in tree-walk.c and unpack-trees.c.  These functions are
designed to take counted strings; in a tuple <a, a_len> they take,
"a_len" is the only thing that determines the length of string "a".
There is no room for a NUL termination inside "a" come into play to
make "a" shorter than "a_len".

In other words, "Two NUL-terminated strings can be compared with
strcmp(a, b), but we use counted strings in many places in our
codebase, and compare_counted_strings(a, a_len, b, b_len) function
would help us, so let's add one and use it in name_compare()" may
make good sense, but if we were to do so, I do not think strncmp()
would be involved in its implementation.

  parent reply	other threads:[~2014-06-17 17:55 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-17  7:34 [PATCH v2 0/3] add strnncmp() function Jeremiah Mahler
2014-06-17  7:34 ` [PATCH v2 1/3] " Jeremiah Mahler
2014-06-17  8:23   ` Torsten Bögershausen
2014-06-17 15:48     ` Jeremiah Mahler
2014-06-17  9:09   ` Erik Faye-Lund
2014-06-17 15:49     ` Jeremiah Mahler
2014-06-17 17:55   ` Junio C Hamano [this message]
2014-06-17 19:27     ` Jeremiah Mahler
2014-06-17  7:34 ` [PATCH v2 2/3] tree-walk: simplify via strnncmp() Jeremiah Mahler
2014-06-17  7:34 ` [PATCH v2 3/3] unpack-trees: " Jeremiah Mahler
2014-06-17 11:08 ` [PATCH v2 0/3] add strnncmp() function Torsten Bögershausen
2014-06-17 15:49   ` Jeremiah Mahler
2014-06-17 17:48     ` Jonathan Nieder
2014-06-17 19:09       ` Jeremiah Mahler
2014-06-18 10:33   ` Ondřej Bílka

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqd2e7mneh.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=jmmahler@gmail.com \
    --cc=jrnieder@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.