From: Eric Sunshine Date: Sat, 20 Feb 2016 08:29:29 GMT Subject: Re: [PATCH 06/21] convert manual allocations to argv_array Message-ID: In-Reply-To: <20160220081010.GA16722@sigill.intra.peff.net> On Sat, Feb 20, 2016 at 3:10 AM, Jeff King wrote: > On Sat, Feb 20, 2016 at 03:07:00AM -0500, Eric Sunshine wrote: >> > + /* argv strings are now owned by pathspec */ >> > + paths.argc = 0; >> > + argv_array_clear(&paths); >> >> This overly intimate knowledge of the internal implementation of >> argv_array_clear() is rather ugly. > > Yep, I agree. Suggestions? > > We can just leak the array of "char *". This function is called only > once per program invocation, and that's unlikely to change. > > I guess we can make an argv_array_detach_strings() function. Or maybe > even just argv_array_detach() would be less gross, and then this > function could manually free the array but not the strings themselves. The latter is what I was thinking, and I agree that argv_array_detach() would be less gross than argv_array_detach_strings(), however, it also feels a bit wrong since this sort of ownership transfer is kind of out of scope for argv_array. I wonder if a simple "dup'ing" string_list would be more suitable for this case. You'd have to append the NULL item manually with string_list_append_nodup(), and string_list_clear() would then be the correct way to dispose of the list without intimate knowledge of its implementation and no need for an API extension.