Re: [PATCH] builtin/receive-pack.c: remove redundant 'if'

Taylor Blau [off-list ref] writes:

The conversion in c7c4bdeccf was mostly plug-and-play, with the snag
that some instances of strvec_pushv() became guarded with a NULL check
to ensure that the second argument was non-NULL.

Thanks.  I am not sure about the reference to plug-and-play but most
of the changes in the commit were mechanical conversion akin to

	@@
	struct child_process C;
	expression V;
	@@
	-C.env = V;
	+strvec_pushv(&C.env_array, V);

but a few have rewritten them to

	@@
	-C.env = V;
	+if (V) strvec_pushv(&C.env_array, V);

And this hunk is one of them.  If we know V is never NULL, the check
becomes redundant, and we can tell tmp_objdir is not NULL at this
point in the control flow.  If C.env (or C.env_array) weren't empty
by the time the control reaches at this point, however, this rewrite
changes behaviour X-<.

Another one in connected.c looks like this:

        diff --git a/connected.c b/connected.c
        index 35bd4a2638..ed3025e7a2 100644
        --- a/connected.c
        +++ b/connected.c
        @@ -109,7 +109,8 @@ int check_connected(oid_iterate_fn fn, void *cb_data,
                                     _("Checking connectivity"));

                rev_list.git_cmd = 1;
        -	rev_list.env = opt->env;
        +	if (opt->env)
        +		strvec_pushv(&rev_list.env_array, opt->env);
                rev_list.in = -1;
                rev_list.no_stdout = 1;
                if (opt->err_fd)

If opt->env can be NULL, the original code would have just cleared
existing rev_list.env but new code instead keeps whatever is in
rev_list.env, which technically speaking changes the behaviour but
we are saved by the fact that rev_list.env hasn't been touched at
this point in the control flow since it was initialized, which makes
it a correct conversion.

There are few others in editor.c::launch_specified_editor(),
run-command.c::run_command_v_opt_cd_env_tr2() and
run-command.c::run_hook_ve(), but the story is identical in these
places.

So, in short, looks good, well spotted, and will apply.

Thanks.