From: Peter Kjellerstedt Date: Tue, 29 Jun 2010 08:15:38 GMT Subject: RE: [PATCH] string-list.h: Add STRING_LIST_INIT macro and make use of it. Message-ID: In-Reply-To: <7v4ogmcs1u.fsf@alter.siamese.dyndns.org> > -----Original Message----- > From: git-owner@vger.kernel.org [mailto:git-owner@vger.kernel.org] On > Behalf Of Junio C Hamano > Sent: den 29 juni 2010 07:43 > To: Alex Riesen > Cc: Thiago Farina; git@vger.kernel.org; jrnieder@gmail.com; > srabbelier@gmail.com > Subject: Re: [PATCH] string-list.h: Add STRING_LIST_INIT macro and make > use of it. > > Alex Riesen writes: > > >> But I wonder if this is worth it (not this "more complete", but > >> your patch). We convert 16 or so instances of initialization for > >> "no strdup" list, but there are about the same number of "strdup" > >> instances still spelled out: > >> > >> $ git grep -e 'struct string_list [^ ]* = {.*' > >> $ git grep -e 'struct string_list [^ ]* = {.*1' > >> > >> Wouldn't it be more sensible to use this instead? > >> > >> #define STRING_LIST_INIT(pleasedup) { NULL, 0, 0, (pleasedup) } > > > > This begs for using strdup(string-to-dup) in the macro argument, > > which will not compile with ancient compilers which don't allow > > code in an initializer. > > Err, one of us must be confused. > > I was suggesting to rewrite things like these, found in > builtin/remote.c (add and rm): > > struct string_list track = { NULL, 0, 0, 0 }; > struct string_list branches = { NULL, 0, 0, 1 }; > > like so: > > #define STRING_LIST_INIT(pleasedup) { NULL, 0, 0, (pleasedup) } > struct string_list track = STRING_LIST_INIT(0); > struct string_list branches = STRING_LIST_INIT(1); > > I don't see strdup in macro argument anywhere. > > The original patch in this thread proposed to do this instead: > > #define STRING_LIST_INIT { NULL, 0, 0, 0 } > struct string_list track = STRING_LIST_INIT; > struct string_list branches = { NULL, 0, 0, 1 }; > > converting about half of initializers to the macro, and leaving the > other half spelled out. How about using two macros instead: #define STRING_LIST_INIT { NULL, 0, 0, 0 } #define STRING_LIST_INIT_DUP { NULL, 0, 0, 1 } That way there is less confusion to what the 0/1 argument to STRING_LIST_INIT() means. (I guess STRING_LIST_INIT could be STRING_LIST_INIT_NODUP instead if one wants more clarity.) //Peter