On Thu, Jul 14, 2016 at 2:35 PM, Jeff King <[email protected]> wrote:
> On Thu, Jul 14, 2016 at 02:24:54PM -0700, Stefan Beller wrote:
>
>> # interdiff to v5:
>> [...giant deletion...]
>
> Much nicer. :)
>
>> +static struct string_list *read_push_options(void)
>> +{
>> + struct string_list *ret = xmalloc(sizeof(*ret));
>
> This struck me as a little non-idiomatic for our code base. The usual
> technique is to take a pointer to a stack-allocated struct, and write
> into that.
Oh, right! :(
That's what you get when you grow up with object orientation all along.
read_push_options() mentally mapped to "create a push options object if any"
such that I can see if it is NULL or not.
I'll reroll a more idiomatic thing. Thanks,
Stefan
>
> I guess that here:
>
>> @@ -1774,6 +1806,9 @@ int cmd_receive_pack(int argc, const char **argv,
>> const char *prefix)
>> const char *unpack_status = NULL;
>> struct string_list *push_options = NULL;
>>
>> + if (use_push_options)
>> + push_options = read_push_options();
>> +
>
> You will want to later check whether push_options is NULL. But you can
> also just check push_options.nr.
>
> -Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
