Re: [PATCH v4 4/5] branch.c: replace `git_config()` with `git_config_get_string()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Tanay Abhra <tanayabh@xxxxxxxxx> writes:

>  int read_branch_desc(struct strbuf *buf, const char *branch_name)
>  {
> -	struct branch_desc_cb cb;
> +	char *v = NULL;
>  	struct strbuf name = STRBUF_INIT;
>  	strbuf_addf(&name, "branch.%s.description", branch_name);
> -	cb.config_name = name.buf;
> -	cb.value = NULL;
> -	if (git_config(read_branch_desc_cb, &cb) < 0) {
> +	if (git_config_get_string(name.buf, &v)) {
>  		strbuf_release(&name);
>  		return -1;
>  	}
> -	if (cb.value)
> -		strbuf_addstr(buf, cb.value);
> +	strbuf_addstr(buf, v);
> +	free(v);
>  	strbuf_release(&name);
>  	return 0;
>  }

I think this is a behavior change. if (git_config(read_branch_desc_cb,
&cb) < 0) was never true in practice, so the "return -1" was essentially
dead code. You now return -1 when no value is found.

It probably doesn't matter, since all caller except
fmt-merge-msg.c:add_branch_desc() ignore the return value, and if I read
correctly, add_branch_desc does not need the test on the return value,
as the then branch of the if does nothing if no value is found anyway.

But here again, I have to wonder why the function does not just return
void.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]