Re: [PATCH v16 Part II 2/8] bisect--helper: `bisect_write` shell function in C

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Martin,

On Fri, 23 Nov 2018, Martin Ågren wrote:

> On Fri, 23 Nov 2018 at 11:13, Johannes Schindelin
> <Johannes.Schindelin@xxxxxx> wrote:
> > On Mon, 30 Oct 2017, Pranit Bauva wrote:
> >
> > > On Fri, Oct 27, 2017 at 10:58 PM, Martin Ågren <martin.agren@xxxxxxxxx> wrote:
> > > > On 27 October 2017 at 17:06, Pranit Bauva <pranit.bauva@xxxxxxxxx> 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);
> > > >> +}
> 
> > > > 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.

I am glad you agree, and it's just fair that I contribute a sanity check
on this here list when I have benefitted so many times from the same.

Ciao,
Johannes

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux