Re: [PATCH v4 05/17] strbuf: add strbuf_isspace()

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jan 31, 2014, 02:51 UTC
Message-ID: <CAPig+cTyAMimdvzbX8hCHQfdd-DPKt4N8LThfYmRbvjeDk6adg@mail.gmail.com>
In-Reply-To: <20140130064921.7504.74582.chriscool@tuxfamily.org>

On Thu, Jan 30, 2014 at 1:49 AM, Christian Couder <chriscool@tuxfamily.org> wrote:

Show 17 quoted lines

> This helper function checks if a strbuf
> contains only space chars or not.
>
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
> diff --git a/strbuf.c b/strbuf.c
> index 83caf4a..2124bb8 100644
> --- a/strbuf.c
> +++ b/strbuf.c
> @@ -124,6 +124,13 @@ void strbuf_ltrim(struct strbuf *sb)
>         sb->buf[sb->len] = '\0';
>  }
>
> +int strbuf_isspace(struct strbuf *sb)
> +{
> +       char *b;
> +       for (b = sb->buf; *b && isspace(*b); b++);

Quoting from the strbuf documentation:

    ... strbufs may have embedded NULs. An strbuf is NUL
    terminated for convenience, but no function in the
    strbuf API actually relies on the string being free of
    NULs.

So, the termination condition (*b) of this loop is questionable. Looping from 0 to < sb->len makes more sense.

> +       return !*b;

Ditto for the return. This will incorrectly return 'true' if an embedded NUL is encountered.

Show 5 quoted lines

> +}
> +
>  struct strbuf **strbuf_split_buf(const char *str, size_t slen,
>                                  int terminator, int max)
>  {