Re: [PATCH v3 14/35] connect: request remote refs using v2

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

 



On Thu, Feb 22, 2018 at 10:19:22AM -0800, Brandon Williams wrote:

> On 02/21, Jonathan Tan wrote:
> > On Tue,  6 Feb 2018 17:12:51 -0800
> > Brandon Williams <bmwill@xxxxxxxxxx> wrote:
> > 
> > > +extern struct ref **get_remote_refs(int fd_out, struct packet_reader *reader,
> > > +				    struct ref **list, int for_push,
> > > +				    const struct argv_array *ref_patterns);
> > 
> > I haven't looked at the rest of this patch in detail, but the type of
> > ref_patterns is probably better as struct string_list, since this is not
> > a true argument array (e.g. with flags starting with --). Same comment
> > for the next few patches that deal with ref patterns.
> 
> Its just a list of strings which don't require having a util pointer
> hanging around so actually using an argv_array would be more memory
> efficient than a string_list.  But either way I don't think it matters
> much.

I agree that it shouldn't matter much here. But if the name argv_array
is standing in the way of using it, I think we should consider giving it
a more general name. I picked that not to evoke "this must be arguments"
but "this is terminated by a single NULL".

In general I think it should be the preferred structure for string
lists, just because it actually converts for free to the "other" common
format (whereas you can never pass string_list.items to a function that
doesn't know about string lists).

-Peff



[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