Re: [PATCH v2 1/1] Don't free remote->name after fetch
- From
- Keith McGuigan <kmcguigan@twopensource.com>
- Date
- Jun 14, 2016, 18:54 UTC
- Message-ID
- <CALnYDJObFtrKFBRbbaA2KV257j8L8gcKcGk-Pkhsyvbk7+oTjQ@mail.gmail.com>
- In-Reply-To
- <xmqqvb1b8v2w.fsf@gitster.mtv.corp.google.com>
Yeah that was the only place I found where it was doing the strdup already (and in that situation, it has to). All the other places just grabbed remote->name.
Yes, sorry, I can sign off on this. Do you want me to resend with the header in place, or is this confirmation good enough?
-- - Keith
On Tue, Jun 14, 2016 at 2:49 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 53 quoted lines
> kmcguigan@twopensource.com writes:
>
>> From: Keith McGuigan <kmcguigan@twopensource.com>
>>
>> Make fetch's string_list of remote names owns all of its string items
>> (strdup'ing when necessary) so that it can deallocate them safely when
>> clearing.
>>
>> ---
>
> OK.
>
> When I pointed out the call to string_list_append() in
> get_remote_group() as one example, I did not check if there are
> others that need similar adjustment. I just skimmed through the
> file again and it seems there is none, so this change looks good.
>
> I assume you meant to sign this off, too?
>
>> builtin/fetch.c | 6 ++----
>> 1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/builtin/fetch.c b/builtin/fetch.c
>> index 630ae6a1bb78..1b4e924bd222 100644
>> --- a/builtin/fetch.c
>> +++ b/builtin/fetch.c
>> @@ -1071,7 +1071,7 @@ static int get_remote_group(const char *key, const char *value, void *priv)
>> size_t wordlen = strcspn(value, " \t\n");
>>
>> if (wordlen >= 1)
>> - string_list_append(g->list,
>> + string_list_append_nodup(g->list,
>> xstrndup(value, wordlen));
>> value += wordlen + (value[wordlen] != '\0');
>> }
>> @@ -1261,7 +1261,7 @@ done:
>> int cmd_fetch(int argc, const char **argv, const char *prefix)
>> {
>> int i;
>> - struct string_list list = STRING_LIST_INIT_NODUP;
>> + struct string_list list = STRING_LIST_INIT_DUP;
>> struct remote *remote;
>> int result = 0;
>> struct argv_array argv_gc_auto = ARGV_ARRAY_INIT;
>> @@ -1347,8 +1347,6 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
>> argv_array_clear(&options);
>> }
>>
>> - /* All names were strdup()ed or strndup()ed */
>> - list.strdup_strings = 1;
>> string_list_clear(&list, 0);
>>
>> close_all_packs();