On Tue, Sep 15, 2015 at 11:25 AM, Jeff King <peff@xxxxxxxx> wrote:
> The strbuf_complete_line function make sure that a buffer
s/make/makes/
> ends in a newline. But we may want to do this for any
> character (e.g., "/" on the end of a path). Let's factor out
> a generic version, and keep strbuf_complete_line as a thin
> wrapper.
>
> Signed-off-by: Jeff King <peff@xxxxxxxx>
> ---
> +/**
> + * Ensure that `sb` ends with the character `term`, if it does not
> + * already.
> + */
> +static inline void strbuf_complete(struct strbuf *sb, char term)
> +{
> + if (sb->len && sb->buf[sb->len - 1] != term)
> + strbuf_addch(sb, term);
> +}
Hmm, so this only adds 'term' if not already present *and* if 'sb' is
not empty, which doesn't seem to match the documentation which says
that it "ensures" termination.
But, is that reasonable behavior? Intuitively, I'd expect 'term' to be
added when 'sb' is empty:
if (!sb->len || sb->buf[sb->len - 1] != term)
strbuf_addch(sb, term);
strbuf_complete_line()'s existing behavior of not adding '\n' to an
empty string may have been intentional, but actually smells like a
bug.
> +
> +/**
> + * Ensure that `sb` ends with a newline.
> + */
> static inline void strbuf_complete_line(struct strbuf *sb)
> {
> - if (sb->len && sb->buf[sb->len - 1] != '\n')
> - strbuf_addch(sb, '\n');
> + strbuf_complete(sb, '\n');
> }
--
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