public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Matthieu Moy <Matthieu.Moy@Grenoble-INP•fr>
To: Tanay Abhra <tanayabh@gmail•com>
Cc: git@vger•kernel.org, Ramkumar Ramachandra <artagnon@gmail•com>,
	Eric Sunshine <sunshine@sunshineco•com>,
	Jeff King <peff@peff•net>, Torsten Bogershausen <tboegi@web•de>
Subject: Re: [PATCH v1] config: Add hashtable for config parsing & retrival
Date: Mon, 09 Jun 2014 16:24:47 +0200	[thread overview]
Message-ID: <vpqmwdmtb2o.fsf@anie.imag.fr> (raw)
In-Reply-To: <1402318146-5062-2-git-send-email-tanayabh@gmail.com> (Tanay Abhra's message of "Mon, 9 Jun 2014 05:49:06 -0700")

Tanay Abhra <tanayabh@gmail•com> writes:

> +the highest priority(i.e. for the same variable value in the repo config
                       ^
missing space.

> +struct config_cache_entry {
> +	struct hashmap_entry ent;
> +	char *key;
> +	struct string_list *value_list;
> +};

I guess this crossed Eric's remark about the fact that this is a
pointer.

> +static int hashmap_is_init;

I'd call it hashmap_initialized, but that's a matter of taste.

> +static void config_cache_free(void)

I didn't look closely enough to make sure there were no memory leak
remaining, but did you check with valgrind --leak-check that it is the
case in practice?

> +	/* contents of config file has changed, so invalidate the
> +	 * config cache used by non-callback based query functions.
> +	 */

Style: Git usually writes multi-line comments

/*
 * like
 * this
 */

(not always applied, but documented in Documentation/CodingGuidelines)

(no time for a more detailed review, sorry)

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

  reply	other threads:[~2014-06-09 14:25 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-09 12:49 [PATCH v1] Git config cache & special querying api utilizing the cache Tanay Abhra
2014-06-09 12:49 ` [PATCH v1] config: Add hashtable for config parsing & retrival Tanay Abhra
2014-06-09 14:24   ` Matthieu Moy [this message]
2014-06-10 11:51     ` Eric Sunshine
2014-06-10 11:27   ` Eric Sunshine
2014-06-10 12:35     ` Tanay Abhra
2014-06-10 21:28       ` Eric Sunshine
2014-06-10 11:45   ` Eric Sunshine
2014-06-10 12:37     ` Tanay Abhra
2014-06-10 23:30   ` Eric Sunshine
2014-06-11 14:01 ` [PATCH v1] Git config cache & special querying api utilizing the cache Matthieu Moy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=vpqmwdmtb2o.fsf@anie.imag.fr \
    --to=matthieu.moy@grenoble-inp$(echo .)fr \
    --cc=artagnon@gmail$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=peff@peff$(echo .)net \
    --cc=sunshine@sunshineco$(echo .)com \
    --cc=tanayabh@gmail$(echo .)com \
    --cc=tboegi@web$(echo .)de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox