Date: Wed, 25 Nov 2009 15:56:39 -0500
From: Bart Trojanowski <bart@jukie.net>
To: Jan Janak <jan@ryngle.com>
Message-ID: <20091125205639.GJ29312@jukie.net>
References: <1259100630-13673-1-git-send-email-jan@ryngle.com>
	<1259100630-13673-2-git-send-email-jan@ryngle.com>
	<1259100630-13673-3-git-send-email-jan@ryngle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1259100630-13673-3-git-send-email-jan@ryngle.com>
User-Agent: Mutt/1.5.18 (2008-05-17)
Cc: notmuch@notmuchmail.org
Subject: Re: [notmuch] [PATCH 2/4] notmuch: Config option to specify tags	to
	be applied by 'notmuch new'.
Precedence: list

Jan,

I really want this feature to get in, so I am going to do my best to
review your code :)

Here are some more sticking points...

> +char **
> +notmuch_config_get_new_tags (notmuch_config_t *config, size_t *length);

If you are not giving over control of the pointer to the caller please
return const char * const *.

Similarly...

> +    char **new_tags;

... this should probably be const char **.

Next...

> +char **
> +notmuch_config_get_new_tags (notmuch_config_t *config, size_t *length)

... but ...

> +    unsigned int count, i;
> +
> +    if ((tags = notmuch_config_get_new_tags (config, &count)) == NULL)
> +	return;

size_t != unsigned int on all platforms.  Please stick with one or the
other.  Note there are a few calls to fix here.

-Bart

-- 
				WebSig: http://www.jukie.net/~bart/sig/