Re: [PATCH 06/21] convert manual allocations to argv_array
- From
Jeff King <peff@peff.net>
- Date
- Feb 20, 2016, 08:10 UTC
- Message-ID
- <20160220081010.GA16722@sigill.intra.peff.net>
- In-Reply-To
- <CAPig+cQsFAd4WEoHP-eoz9HCcVzY2uVcDH+_rvkoNjSe-Fg0oA@mail.gmail.com>
On Sat, Feb 20, 2016 at 03:07:00AM -0500, Eric Sunshine wrote:
Show 32 quoted lines
> > diff --git a/line-log.c b/line-log.c
> > @@ -746,23 +747,16 @@ void line_log_init(struct rev_info *rev, const char *prefix, struct string_list
> > if (!rev->diffopt.detect_rename) {
> > - int i, count = 0;
> > - struct line_log_data *r = range;
> > - const char **paths;
> > - while (r) {
> > - count++;
> > - r = r->next;
> > - }
> > - paths = xmalloc((count+1)*sizeof(char *));
> > - r = range;
> > - for (i = 0; i < count; i++) {
> > - paths[i] = xstrdup(r->path);
> > - r = r->next;
> > - }
> > - paths[count] = NULL;
> > + struct line_log_data *r;
> > + struct argv_array paths = ARGV_ARRAY_INIT;
> > +
> > + for (r = range; r; r = r->next)
> > + argv_array_push(&paths, r->path);
> > parse_pathspec(&rev->diffopt.pathspec, 0,
> > - PATHSPEC_PREFER_FULL, "", paths);
> > - free(paths);
> > + PATHSPEC_PREFER_FULL, "", paths.argv);
> > + /* 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.
-Peff