On Mon, Jun 23, 2014 at 6:41 AM, Tanay Abhra <tanayabh@xxxxxxxxx> wrote:
> Use git_config_get_string instead of git_config to take advantage of
> the config hash-table api which provides a cleaner control flow.
>
> Signed-off-by: Tanay Abhra <tanayabh@xxxxxxxxx>
> ---
> alias.c | 28 ++++++++++------------------
> 1 file changed, 10 insertions(+), 18 deletions(-)
>
> diff --git a/alias.c b/alias.c
> index 5efc3d6..0fe32bc 100644
> --- a/alias.c
> +++ b/alias.c
> @@ -1,25 +1,17 @@
> #include "cache.h"
>
> -static const char *alias_key;
> -static char *alias_val;
> -
> -static int alias_lookup_cb(const char *k, const char *v, void *cb)
> -{
> - if (starts_with(k, "alias.") && !strcmp(k + 6, alias_key)) {
> - if (!v)
> - return config_error_nonbool(k);
> - alias_val = xstrdup(v);
> - return 0;
> - }
> - return 0;
> -}
> -
> char *alias_lookup(const char *alias)
> {
> - alias_key = alias;
> - alias_val = NULL;
> - git_config(alias_lookup_cb, NULL);
> - return alias_val;
> + const char *v;
> + char *value;
> + struct strbuf key = STRBUF_INIT;
> + strbuf_addf(&key, "alias.%s", alias);
> + git_config_get_string(key.buf, &v);
> + if (!v)
> + config_error_nonbool(key.buf);
If 'v' is NULL, you correctly report an error, but then fall through
and invoke xstrdup() with NULL, which invites undefined behavior [1].
[1]: http://pubs.opengroup.org/onlinepubs/009695399/functions/strdup.html
> + value = xstrdup(v);
> + strbuf_release(&key);
> + return value;
You could release the strbuf earlier, which would allow you to 'return
xstrdup(v)' and drop the 'value' variable. Perhaps you want something
like this:
const char *v;
struct strbuf key = STRBUF_INIT;
strbuf_addf(&key, "alias.%s", alias);
git_config_get_string(key.buf, &v);
if (v)
config_error_nonbool(key.buf);
strbuf_release(&key);
return v ? xstrdup(v) : NULL;
> }
>
> #define SPLIT_CMDLINE_BAD_ENDING 1
> --
> 1.9.0.GIT
--
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