Re: [PATCH v2 04/11] sparse-checkout: 'set' subcommand

From: Elijah Newren <newren@gmail.com>
Date: Oct 5, 2019, 22:44 UTC
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:

Show 12 quoted lines

> +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.