From: Thiago Farina <tfransosi@gmail.com>
Date: Thu, 09 Dec 2010 15:58:28 GMT
Subject: Re: [PATCH 2/3] config: add git_config_from_sha1() to read from a blob
Message-ID: <AANLkTi=qt-uwXdUquRAv_1VpMUQuUR+aNMuxSHdpMhXd@mail.gmail.com>
In-Reply-To: <1291907388-9068-3-git-send-email-pclouds@gmail.com>

2010/12/9 Nguyễn Thái Ngọc Duy <pclouds@gmail.com>:
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  config.c |   34 ++++++++++++++++++++++++++++++++++
>  1 files changed, 34 insertions(+), 0 deletions(-)
>
> diff --git a/config.c b/config.c
> index c8bf46f..e7a9ff4 100644
> --- a/config.c
> +++ b/config.c
> @@ -809,6 +809,40 @@ int git_config_from_file(config_fn_t fn, const char *filename, void *data)
>        return ret;
>  }
>

> +static int git_config_from_sha1(config_fn_t fn, const char *sha1_name, void *data)
> +{

Is worth documenting the return value of this function and what it
does? It returns 0 on success otherwise returns -1.

> +       unsigned char sha1[20];
> +       enum object_type type;
> +       unsigned long size;
> +       int ret;
> +
> +       if (get_sha1(sha1_name, sha1)) {
> +               error("bad shared config reference '%s'", sha1_name);
> +               return -1;
> +       }
> +
> +       config_file = read_sha1_file(sha1, &type, &size);
> +       if (!config_file) {
> +               error("bad shared config '%s'", sha1_name);
> +               return -1;
> +       }
> +       if (type == OBJ_BLOB) {
> +               config_file_pos = 0;
> +               config_file_size = size;
> +               config_file_name = sha1_name;
> +               config_linenr = 1;
> +               config_file_eof = 0;
> +               ret = git_parse_file(fn, data);
> +       }
> +       else {

style nit: Shouldn't this else be on the end of the previous line?