Re: [PATCH v3 2/7] bisect: read bisect paths with strbuf_getline()

[Eric Sunshine <sunshine@sunshineco.com>]

On Sun, Feb 28, 2016 at 12:13 AM, Moritz Neeb <lists@moritzneeb.de> wrote:
> The file BISECT_NAMES is written by "git rev-parse --sq-quote" via
> sq_quote_argv() when starting a bisection. It can contain pathspecs
> to narrow down the search. When reading it back, it should be expected that
> sq_dequote_to_argv_array() is able to parse this file. In fact, the
> previous commit ensures this.
>
> As the content is of type "text", that means there is no logic expecting
> CR, strbuf_getline_lf() will be replaced by strbuf_getline().
>
> Apart from whitespace added and removed in quote.c, no more whitespaces
> are expexted. While it is technically possible, we have never advertised

s/expexted/expected/

> this file to be editable by user, or encouraged them to do so, thus
> the call to strbuf_trim() turns obsolete in various ways.

Not sure what "various ways" you mean. Perhaps say instead that (as a
consequence of "not advertised or encouraged") you're tightening the
parsing of this file by removing strbuf_trim().

> For the case that this file is modified nonetheless, in an invalid way
> such that dequoting fails, the error message is broadened to both cases:
> bad quoting and unexpected whitespace.
>
> Helped-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Moritz Neeb <lists@moritzneeb.de>
> ---
> diff --git a/bisect.c b/bisect.c
> @@ -440,10 +440,9 @@ static void read_bisect_paths(struct argv_array *array)
>         if (!fp)
>                 die_errno("Could not open file '%s'", filename);
>
> -       while (strbuf_getline_lf(&str, fp) != EOF) {
> -               strbuf_trim(&str);
> +       while (strbuf_getline(&str, fp) != EOF) {
>                 if (sq_dequote_to_argv_array(str.buf, array))
> -                       die("Badly quoted content in file '%s': %s",
> +                       die("Badly quoted content or unexpected whitespace in file '%s': %s",
>                             filename, str.buf);
>         }
>
> --
> 2.4.3