On Sun, Feb 14, 2016 at 10:53 PM, Duy Nguyen <pclouds@xxxxxxxxx> wrote:
> On Fri, Feb 5, 2016 at 12:26 PM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
>> On Thu, Feb 4, 2016 at 4:03 AM, Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> wrote:
>>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
>>> ---
>>> diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
>>> @@ -109,6 +109,16 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
>>> + if (skip_prefix(arg, "--shallow-exclude=", &value)) {
>>> + static struct string_list *deepen_not;
>>> + if (!deepen_not) {
>>> + deepen_not = xmalloc(sizeof(*deepen_not));
>>> + string_list_init(deepen_not, 1);
>>> + args.deepen_not = deepen_not;
>>> + }
>>> + string_list_append(deepen_not, value);
>>> + continue;
>>> + }
>>
>> Hmm, can't this be simplified to:
>>
>> if (skip_prefix(arg, "--shallow-exclude=", &value)) {
>> if (!args.deepen_not) {
>> args.deepen_not = xmalloc(sizeof(*args.deepen_not));
>> string_list_init(args.deepen_not, 1);
>> }
>> string_list_append(args.deepen_not, value);
>> continue;
>> }
>
> args.deepen_not is const, so no, the compiler will complain at
> string_list_init and string_list_append. Dropping "const" is one
> option, if you prefer.
Yes, dropping 'const' was implied. I didn't examine it too deeply, but
it did not appear as if there would be any major fallout from dropping
'const'. It feels a bit cleaner to keep it all self-contained than to
have that somewhat oddball static string_list*, but it's not such a
big deal that I'd insist upon a rewrite.
>> Or, perhaps even better, declare it as plain 'struct string_list
>> deepen_not' in struct fetch_pack_args, rather than as a pointer, and
>> then in cmd_fetch_pack():
>>
>> memset(&args, 0, sizeof(args));
>> args.uploadpack = "git-upload-pack";
>> string_list_init(&args.deepen_not, 1);
>
> There's another place fetch_pack_args variable is declared, in
> fetch_refs_via_pack(), and we would need to string_list_copy() from
> transport->data->options.deepen_not and then free it afterward. So I
> think it's not really worth it.
Okay.
>> if (skip_prefix(arg, "--shallow-exclude=", &value)) {
>> string_list_append(args.deepen_not, value);
>> continue;
>> }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html