config: use xstrfmt in normalize_value

We xmalloc a fixed-size buffer and sprintf into it; this is
OK because the size of our formatting types is finite, but
that's not immediately clear to a reader auditing sprintf
calls. Let's switch to xstrfmt, which is shorter and
obviously correct.

Note that just dropping the common xmalloc here causes gcc
to complain with -Wmaybe-uninitialized. That's because if
"types" does not match any of our known types, we never
write anything into the "normalized" pointer. With the
current code, gcc doesn't notice because we always return a
valid pointer (just one which might point to uninitialized
data, but the compiler doesn't know that). In other words,
the current code is potentially buggy if new types are added
without updating this spot.

So let's take this opportunity to clean up the function a
bit more. We can drop the "normalized" pointer entirely, and
just return directly from each code path. And then add an
assertion at the end in case we haven't covered any cases.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/config.c b/builtin/config.c
index 71acc4414352e8d097447993937db96a1cc6efaa..adc772786a7ddf9952aaccb7578b57d10396da93 100644 (file)
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -246,8 +246,6 @@ static int get_value(const char *key_, const char *regex_)
 
 static char *normalize_value(const char *key, const char *value)
 {
-       char *normalized;
-
        if (!value)
                return NULL;
 
@@ -258,27 +256,21 @@ static char *normalize_value(const char *key, const char *value)
                 * "~/foobar/" in the config file, and to expand the ~
                 * when retrieving the value.
                 */
-               normalized = xstrdup(value);
-       else {
-               normalized = xmalloc(64);
-               if (types == TYPE_INT) {
-                       int64_t v = git_config_int64(key, value);
-                       sprintf(normalized, "%"PRId64, v);
-               }
-               else if (types == TYPE_BOOL)
-                       sprintf(normalized, "%s",
-                               git_config_bool(key, value) ? "true" : "false");
-               else if (types == TYPE_BOOL_OR_INT) {
-                       int is_bool, v;
-                       v = git_config_bool_or_int(key, value, &is_bool);
-                       if (!is_bool)
-                               sprintf(normalized, "%d", v);
-                       else
-                               sprintf(normalized, "%s", v ? "true" : "false");
-               }
+               return xstrdup(value);
+       if (types == TYPE_INT)
+               return xstrfmt("%"PRId64, git_config_int64(key, value));
+       if (types == TYPE_BOOL)
+               return xstrdup(git_config_bool(key, value) ?  "true" : "false");
+       if (types == TYPE_BOOL_OR_INT) {
+               int is_bool, v;
+               v = git_config_bool_or_int(key, value, &is_bool);
+               if (!is_bool)
+                       return xstrfmt("%d", v);
+               else
+                       return xstrdup(v ? "true" : "false");
        }
 
-       return normalized;
+       die("BUG: cannot normalize type %d", types);
 }
 
 static int get_color_found;