Re: [PATCH 2/9] add strip_suffix function
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 2, 2014, 15:54 UTC
- Message-ID
- <xmqqlhsbiwmz.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <20140630165751.GB16637@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 5 quoted lines
> For that reason, the "mem" form puts its length parameter > next to the buffer (since they are a pair), and the string > form puts it at the end (since it is an out-parameter). The > compiler can notice when you get the order wrong, which > should help prevent writing one when you meant the other.
Very sensible consideration. I like commits that careful thinking behind them shows through them.
Show 7 quoted lines
> Signed-off-by: Jeff King <peff@peff.net> > --- > I hope the word "strip" is OK, as it does not actually NUL-terminate > (doing so would make it unusable for many cases). Between the comment > below and the "const" in the parameter, I think it should be pretty > clear that it does not touch the string. And I could not think of a > better word.
All other words I can think of offhand, trim, chomp, etc., hint shortening of the input string, and by definition shortening of a string implies NUL-termination.
The "mem" variant deals with a counted string, however, so its shortening implies NUL-termination a lot less [*1*] and should be fine.
If we want to avoid implying NUL-termination, the only way to do so would be to use wording that does not hint shortening. At least for the C-string variant, which is measuring the length of the basename part (i.e. `basename $str $suffix`) without touching anything else, e.g. basename_length("hello.c", ".c", &len), but at the same time you want to make it a boolean to signal if the string ends with the suffix, so perhaps has_suffix("hello.c", ".c", &len)?
[Footnote]
*1* ... but not entirely, because we often NUL-terminate even
counted strings (the buffer returned from read_sha1_file() and
the payload of strbuf are two examples).Show 41 quoted lines
> git-compat-util.h | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/git-compat-util.h b/git-compat-util.h
> index b6f03b3..d044c42 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -358,6 +358,33 @@ static inline const char *skip_prefix(const char *str, const char *prefix)
> return NULL;
> }
>
> +/*
> + * If buf ends with suffix, return 1 and subtract the length of the suffix
> + * from *len. Otherwise, return 0 and leave *len untouched.
> + */
> +static inline int strip_suffix_mem(const char *buf, size_t *len,
> + const char *suffix)
> +{
> + size_t suflen = strlen(suffix);
> + if (*len < suflen || memcmp(buf + (*len - suflen), suffix, suflen))
> + return 0;
> + *len -= suflen;
> + return 1;
> +}
> +
> +/*
> + * If str ends with suffix, return 1 and set *len to the size of the string
> + * without the suffix. Otherwise, return 0 and set *len to the size of the
> + * string.
> + *
> + * Note that we do _not_ NUL-terminate str to the new length.
> + */
> +static inline int strip_suffix(const char *str, const char *suffix, size_t *len)
> +{
> + *len = strlen(str);
> + return strip_suffix_mem(str, len, suffix);
> +}
> +
> #if defined(NO_MMAP) || defined(USE_WIN32_MMAP)
>
> #ifndef PROT_READ