Re: [RFC/PATCH V2] alias.c: replace git_config with git_config_get_string

2014-06-26 Thread Tanay Abhra 


On 6/25/2014 7:42 AM, Eric Sunshine wrote:
 On Mon, Jun 23, 2014 at 6:41 AM, Tanay Abhra tanay...@gmail.com 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 tanay...@gmail.com
 ---
  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;
 

Done. Thanks.

  }

  #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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH V2] alias.c: replace git_config with git_config_get_string

2014-06-26 Thread Matthieu Moy 
Tanay Abhra tanay...@gmail.com writes:

 the config hash-table api which provides a cleaner control flow.

api - API

-- 
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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH V2] alias.c: replace git_config with git_config_get_string

2014-06-24 Thread Eric Sunshine 
On Mon, Jun 23, 2014 at 6:41 AM, Tanay Abhra tanay...@gmail.com 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 tanay...@gmail.com
 ---
  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC/PATCH V2] alias.c: replace git_config with git_config_get_string

2014-06-23 Thread Tanay Abhra 
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 tanay...@gmail.com
---
 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);
+   value = xstrdup(v);
+   strbuf_release(key);
+   return value;
 }
 
 #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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH V2] alias.c: replace git_config with git_config_get_string

2014-06-23 Thread Jonathan Nieder 
Tanay Abhra wrote:

  alias.c | 28 ++--
  1 file changed, 10 insertions(+), 18 deletions(-)

What commit are these patches against?  Are they a continuation
of the git config cache  special querying api series?

Thanks,
Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH V2] alias.c: replace git_config with git_config_get_string

2014-06-23 Thread Tanay Abhra 

On 6/24/2014 4:08 AM, Jonathan Nieder wrote:

Tanay Abhra wrote:


  alias.c | 28 ++--
  1 file changed, 10 insertions(+), 18 deletions(-)


What commit are these patches against?  Are they a continuation
of the git config cache  special querying api series?



My fault, I should have marked them. You have inferred correctly,
they have been built on top of git config cache  special querying
api series[1].

[1] http://thread.gmane.org/gmane.comp.version-control.git/252329

Thanks,
Tanay.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html