Re: [PATCH v6] fetch-pack: always allow fetching of literal SHA1s

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

 



Jeff King <peff@xxxxxxxx> writes:

> On Fri, May 12, 2017 at 01:46:48PM -0700, Jonathan Tan wrote:
>
>> Change from v5: used "ensure_tip_oids_initialized" function instead.
>> This removes some of the muddiness (e.g. with newlist being modified
>> after the function).
>
> I don't think it really improves the muddiness. You are still calling
> the ensure function each time through the loop with a potentially
> modified list, but it doesn't actually look at the list after the first
> time. So the muddy part is still there.
>
> I think rather than changing the code, you'd do better with a comment
> like:
>
>   /*
>    * Note that this only looks at the ref lists the first time
>    * it's called. This works out in filter_refs() because even
>    * though it may add to "newlist" between calls, the additions
>    * will always be for oids that are already in the set.
>    */
>
> At which point the original all-in-one function is probably fine (as it
> avoids the "return 1 just to join the &&-chain" bit).

Yes.  I agree that the in-code comment is the best approach to the
"muddy combination of 'grabbing it for the first time' and 'the
thing that is grabbed mutates in the loop'" confusion.




[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]