All of lore.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.fr \
    --cc=artagnon@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    --cc=sunshine@sunshineco.com \
    --cc=tanayabh@gmail.com \
    --cc=tboegi@web.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.