On Thu, 22 Feb 2018 13:26:58 -0500 Jeff King <peff@xxxxxxxx> wrote: > 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). This sounds reasonable - I withdraw my comment about using struct string_list.
