Re: [PATCH v4 4/5] branch.c: replace `git_config()` with `git_config_get_string()
- From
Matthieu Moy <matthieu.moy@grenoble-inp.fr>
- Date
- Jul 30, 2014, 16:23 UTC
- Message-ID
- <vpqr412q0ht.fsf@anie.imag.fr>
- In-Reply-To
- <1406727549-22334-5-git-send-email-tanayabh@gmail.com>
Tanay Abhra <tanayabh@gmail.com> writes:
Show 20 quoted lines
> 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/