Re: [PATCH 3/3] setup: add `clear_repository_format()`

From: "Martin Ågren" <martin.agren@gmail.com>
To: Jeff King <peff@peff.net>
Cc: Git Mailing List <git@vger.kernel.org>,
	"brian m . carlson" <sandals@crustytoothpaste.net>
Subject: Re: [PATCH 3/3] setup: add `clear_repository_format()`
Date: Wed, 19 Dec 2018 22:49:40 +0100	[thread overview]
Message-ID: <CAN0heSr5KFK02XHk2rK39zEUoyoJCkqA=22qSkETehyKewM7GQ@mail.gmail.com> (raw)
In-Reply-To: <20181219154853.GC14802@sigill.intra.peff.net>

On Wed, 19 Dec 2018 at 16:48, Jeff King <peff@peff.net> wrote:
>
> On Tue, Dec 18, 2018 at 08:25:28AM +0100, Martin Ågren wrote:
>
> > +void clear_repository_format(struct repository_format *format)
> > +{
> > +     string_list_clear(&format->unknown_extensions, 0);
> > +     free(format->work_tree);
> > +     free(format->partial_clone);
> > +     memset(format, 0, sizeof(*format));
> >  }
>
> For the callers that actually pick the values out, I think it might be a
> little less error-prone if they actually copied the strings and then
> called clear_repository_format(). That avoids leaks of values that they
> didn't know or care about (and the cost of an extra strdup for
> repository setup is not a big deal).
>
> Something like this on top of your patch, I guess (with the idea being
> that functions which return an error would clear the format, but a
> "successful" one would get returned back up the stack to
> setup_git_directory_gently(), which then clears it before returning.

Thanks for the suggestion. I'll ponder 1) how to go about this
robustifying, 2) how to present the result as part of a v2 series.

To Junio on the sidelines in a cast (hope you're feeling better!):
you can expect a v2 of this series.

Martin