On Mon, Feb 15, 2016 at 12:52 AM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
> 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:
>>> 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.
Upon re-reading, I suppose this also is an argument for keeping the
static string_list* rather than dropping the 'const'...(?)
--
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