Re: [PATCH v16 Part II 2/8] bisect--helper: `bisect_write` shell function in C
- From
- Martin Ågren <martin.agren@gmail.com>
- Date
- Nov 23, 2018, 12:22 UTC
- Message-ID
- <CAN0heSpaW5C_dMh7gh0ezyzACBgZ2SYXP67E_3moeKNyTvu9xg@mail.gmail.com>
- In-Reply-To
- <nycvar.QRO.7.76.6.1811231111030.41@tvgsbejvaqbjf.bet>
On Fri, 23 Nov 2018 at 11:13, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
Show 11 quoted lines
> On Mon, 30 Oct 2017, Pranit Bauva wrote:
>
> > On Fri, Oct 27, 2017 at 10:58 PM, Martin Ågren <martin.agren@gmail.com> wrote:
> > > On 27 October 2017 at 17:06, Pranit Bauva <pranit.bauva@gmail.com> wrote:
> > >> +static void free_terms(struct bisect_terms *terms)
> > >> +{
> > >> + if (!terms->term_good)
> > >> + free((void *) terms->term_good);
> > >> + if (!terms->term_bad)
> > >> + free((void *) terms->term_bad);
> > >> +}Show 14 quoted lines
> > > You leave the pointers dangling, but you're ok for now since this is the > > > last thing that happens in `cmd_bisect__helper()`. Your later patches > > > add more users, but they're also ok, since they immediately assign new > > > values. > > > > > > In case you (and others) find it useful, the below is a patch I've been > > > sitting on for a while as part of a series to plug various memory-leaks. > > > `FREE_AND_NULL_CONST()` would be useful in precisely these situations. > > > > Honestly, I wouldn't be the best person to judge this. > > Git's source code implicitly assumes that any `const` pointer refers to > memory owned by another code path. It is therefore probably not a good > idea to encourage `free()`ing a `const` pointer.
Yeah, I never submitted that patch as part of a real series. I remember having a funky feeling about it, and whatever use-case I had for this macro, I managed to solve the memory leak in some other way in a rewrite.
Thanks for a sanity check.
Martin