Re: [PATCH v4 2/2] sequencer: fix quoting in write_author_script

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Aug 7, 2018, 10:23 UTC
Message-ID: <CAPig+cQkY1_qDu=fdbA0gVPH88B47bM5BpgbYY8ivsj1Y0QHTw@mail.gmail.com>
In-Reply-To: <20180807093452.22524-3-phillip.wood@talktalk.net>

On Tue, Aug 7, 2018 at 5:35 AM Phillip Wood <phillip.wood@talktalk.net> wrote:

Show 5 quoted lines

>      - Reverted the implementation to v2 with more robust detection of the
>        missing "'" on the last line of the author script based on a
>        suggestion by Eric. This means that this series needs to progress
>        closely with Eric's series of fixes or the fallback code will never
>        be called.

Thanks for working on this. I haven't read the patch closely yet, but one thing caught my attention as I ran my eye over it...

Show 7 quoted lines

> +static int quoting_is_broken(const char *s, size_t n)
> +{
> +       /* Skip any empty lines in case the file was hand edited */
> +       while (n > 0 && s[--n] == '\n')
> +               ; /* empty */
> +       if (n > 0 && s[n] != '\'')
> +               return 1;

To be "technically correct", I think the condition in the 'if' statement should be ">=". It should never happen in practice that the entire content of the file is a single character followed by zero or more newlines, but using the proper condition ">=" would save future readers of this code a "huh?" moment.