From: Christian Couder <christian.couder@gmail.com>
Date: Sat, 05 Aug 2017 16:55:53 GMT
Subject: Re: [GSoC][PATCH 08/13] submodule: port submodule subcommand 'summary' from shell to C
Message-ID: <CAP8UFD2oqdaQqVwuGqqj9OOnm5Pyufpck=rod0UFQEWrROJsXQ@mail.gmail.com>
In-Reply-To: <CAME+mvUAspzuzbB4pFiQeOtT7NGRTO25M-k8o8hoysMFLGy_0w@mail.gmail.com>

On Sat, Aug 5, 2017 at 12:28 PM, Prathamesh Chavan <pc44800@gmail.com> wrote:
> On Tue, Aug 1, 2017 at 4:57 AM, Christian Couder
> <christian.couder@gmail.com> wrote:
>> On Mon, Jul 31, 2017 at 10:56 PM, Prathamesh Chavan <pc44800@gmail.com> wrote:
>>
> We can avoid it to behave same for "" and NULL, by checking if diff_cmd
> is "cmd_diff_files", since its value is set NULL by this case.
>
> ret = compute_summary_module_list(strcmp(diff_cmd, "diff-files") ?
> NULL: sb.buf, &info);
> strbuf_release(&sb);

It looks error prone, more fagile and less efficient to me.

> instead of:
> ret = compute_summary_module_list(sb.len ? sb.buf : NULL, &info);
> if (sb.len)
> strbuf_release(&sb);

I think it is ok to call strbuf_release(&sb) many times, so the "if
(sb.len)" check above is not needed.