From: Jeff Hostetler <git@jeffhostetler.com>
Date: Thu, 16 Sep 2021 15:35:51 GMT
Subject: Re: [PATCH 1/7] trace2: fix memory leak of thread name
Message-ID: <285a279f-85c4-06df-04cf-b998e4accbe5@jeffhostetler.com>
In-Reply-To: <87ilz1gd24.fsf@evledraar.gmail.com>



On 9/16/21 4:01 AM, Ævar Arnfjörð Bjarmason wrote:
> 
> On Thu, Sep 16 2021, Taylor Blau wrote:
> 
>> On Thu, Sep 16, 2021 at 07:35:59AM +0200, Ævar Arnfjörð Bjarmason wrote:
>>> So I think this patch can be dropped from this series, since it's exact
>>> duplicate of my 48f68715b14 (tr2: stop leaking "thread_name" memory,
>>> 2021-08-27) in ab/tr2-leaks-and-fixes, currently in "next" and marked
>>> for a merge with master.
>>
>> I agree it can be dropped.
>>
>>> When submitting a series that depends on another one it's best to rebase
>>> it on top of it & indicate it as such in the cover letter, Junio can
>>> queue such a series on top of another one.
>>>
>>> In this case I'm still not sure why this fix is here, i.e. surely
>>> nothing later in the series absolutely needs this stray memory leak
>>> fix...
>>
>> But there's no need for Jeff to depend on your branch, since (as you
>> mentioned) this cleanup isn't relevant for anything else in this series,
>> which is a sort of grab-bag of miscellaneous clean-ups.
> 
> Indeed, to be clear it was just general advice about queue-on-top.
> 
> But to clarify what I was getting at here: If we just came up with the
> same diff I'd have assumed Jeff just hadn't need the change in "next",
> but since he clearly has I was confused by it being here.
> 
> I.e. it doesn't *seem* like anything in the rest of the series depends
> on it, so why have it here at all since the bug is being fixed anyway?
> Or if it does depend on it in some subtle way I've missed, perhaps it
> does need to be queued on top of ab/tr2-leaks-and-fixes, and the
> relevant commit/subtle dependency needs to be called out in a commit
> message.
> 
> Or maybe Jeff had just come up with this independently, noticed it just
> before submission and just updated the CL, not the patch or series
> itself :)
> 

I'll drop this commit since your version is already queued up
and headed to master.  I've been carrying it in my dev branch
for a while and was using it to make leak reporting a little
quieter.

And yes, I just noticed that yours had advanced when I wrote the
cover letter and ACKd it rather than dropping it.

And no, nothing in the rest of the whole FSMonitor series depends
on this, so I can leave my series based upon master rather than
your branch.

Thanks
Jeff