From: Ping Yin <pkufranky@gmail.com>
Date: Sun, 13 Apr 2008 07:28:20 GMT
Subject: Re: [PATCH v3 1/4] config.c: Add git_config_bool_or_int to handle bool/int variable
Message-ID: <46dff0320804130028l76c69b55jb8d6be79125b6041@mail.gmail.com>
In-Reply-To: <7vmynye0cz.fsf@gitster.siamese.dyndns.org>

On Sun, Apr 13, 2008 at 2:26 PM, Junio C Hamano <junio@pobox.com> wrote:
> Ping Yin <pkufranky@gmail.com> writes:
>
>  > With git_config_bool_or_int, the caller can differentiate boolean true
>  > and integer 1 etc.
>  >
>  > Signed-off-by: Ping Yin <pkufranky@gmail.com>
>  > ---
>  >  config.c |   15 +++++++++++++++
>  >  1 files changed, 15 insertions(+), 0 deletions(-)
>  >
>  > diff --git a/config.c b/config.c
>  > index 0624494..e614456 100644
>  > --- a/config.c
>  > +++ b/config.c
>  > @@ -316,6 +316,21 @@ int git_config_bool(const char *name, const char *value)
>  >       return git_config_int(name, value) != 0;
>  >  }
>  >
>  > +int git_config_bool_or_int(const char *name, const char *value, int *is_bool)
>  > +{
>  > +     *is_bool = 1;
>  > +     if (!value)
>  > +             return 1;
>  > +     if (!*value)
>  > +             return 0;
>  > +     if (!strcasecmp(value, "true") || !strcasecmp(value, "yes"))
>  > +             return 1;
>  > +     if (!strcasecmp(value, "false") || !strcasecmp(value, "no"))
>  > +             return 0;
>  > +     *is_bool = 0;
>  > +     return git_config_int(name, value);
>  > +}
>  > +
>
>  I expected git_config_bool() to be implemented in terms of this new
>  function to avoid code duplication if we were actually going to do this.
>
>  You also need an external declaration in a header file for its users.
>
>  ---
>   cache.h  |    1 +
>   config.c |   10 +++++++++-
>   2 files changed, 10 insertions(+), 1 deletions(-)
>
>  diff --git a/cache.h b/cache.h
>  index 2a1e7ec..50b28fa 100644
>  --- a/cache.h
>  +++ b/cache.h
>  @@ -692,6 +692,7 @@ extern int git_parse_long(const char *, long *);
>   extern int git_parse_ulong(const char *, unsigned long *);
>   extern int git_config_int(const char *, const char *);
>   extern unsigned long git_config_ulong(const char *, const char *);
>  +extern int git_config_bool_or_int(const char *, const char *, int *);
>   extern int git_config_bool(const char *, const char *);
>   extern int git_config_string(const char **, const char *, const char *);
>   extern int git_config_set(const char *, const char *);
>
> diff --git a/config.c b/config.c
>  index 0624494..5ea18ef 100644
>
> --- a/config.c
>  +++ b/config.c
>  @@ -303,8 +303,9 @@ unsigned long git_config_ulong(const char *name, const char *value)
>         return ret;
>   }
>
>  -int git_config_bool(const char *name, const char *value)
>
> +int git_config_bool_or_int(const char *name, const char *value, int *is_bool)
>   {
>  +       *is_bool = 1;
>         if (!value)
>                 return 1;
>         if (!*value)
>  @@ -313,9 +314,16 @@ int git_config_bool(const char *name, const char *value)
>
>                 return 1;
>         if (!strcasecmp(value, "false") || !strcasecmp(value, "no"))
>                 return 0;
>  +       *is_bool = 0;
>
>         return git_config_int(name, value) != 0;
>   }
>
>  +int git_config_bool(const char *name, const char *value)
>  +{
>  +       int discard;
>  +       return git_config_bool_or_int(name, value, &discard);

Is it better to use
+       return git_config_bool_or_int(name, value, &discard) != 0;

since git_config_bool_or_int(name, value, &discard) may return an
interger neither 0 nor 1


-- 
Ping Yin