From: Elijah Newren <newren@gmail.com>
Date: Sat, 05 Oct 2019 22:44:10 GMT
Subject: Re: [PATCH v2 04/11] sparse-checkout: 'set' subcommand
Message-ID: <CABPp-BHB0UM1G2tHQ7igRX7Sn1O9ujW6mq_e-UGanGMONHxiag@mail.gmail.com>
In-Reply-To: <9a78f9ea0fe8d1988654f52a86a01031607621fe.1568904188.git.gitgitgadget@gmail.com>

On Thu, Sep 19, 2019 at 3:07 PM Derrick Stolee via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> +static int write_patterns_and_update(struct pattern_list *pl)
> +{
> +       char *sparse_filename;
> +       FILE *fp;
> +
> +       sparse_filename = get_sparse_checkout_filename();
> +       fp = fopen(sparse_filename, "w");
> +       write_patterns_to_file(fp, pl);
> +       fclose(fp);
> +       free(sparse_filename);
> +
> +       clear_pattern_list(pl);

It seems slightly odd that pl is passed in but cleared in this
function rather than in the caller that created pl.  Should this be
moved to the caller, or, alternatively, a comment added to explain
this side-effect for future callers of the function?

The rest of the patch looked good to me.