Re: [PATCH 04/22] builtin/push: fix leaking refspec query result

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

 



On Fri, Aug 30, 2024 at 02:59:01PM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@xxxxxx> writes:
> 
> > When appending a refspec via `refspec_append_mapped()` we leak the
> > result of `query_refspecs()`. The overall logic around refspec queries
> > is quite weird, as callers are expected to either set the `src` or `dst`
> > pointers, and then the (allocated) result will be in the respective
> > other struct member.
> 
> Hmph, is it necessary to say "quite weird" for the purpose of this
> change?  The query interface is designed to be usable to query both
> ways and within that constraints, I find it designed very nicely
> (but I do not think that is necessary to be said for the purpose of
> this change, either)..

I don't quite agree that it's nicely designed -- I find it rather hard
to use and reason about, and the fact that so many callsites get this
interface wrong seems to indicate that there is at least some sort of
truth to this assessment.

But I don't mind removing this subjective opinion from the commit
message. I'll do that in case I end up rerolling this patch series.

Thanks!

Patrick




[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