Hi, I have looked into this because it's marked as "ready for committer".
> On Tue, Nov 19, 2013 at 2:06 PM, Haribabu kommi > <haribabu.ko...@huawei.com> wrote: >> On 19 November 2013 09:59 Amit Kapila wrote: >>> On Mon, Nov 18, 2013 at 8:31 PM, Haribabu kommi >>> <haribabu.ko...@huawei.com> wrote: >>> > On 18 November 2013 20:01 Amit Kapila wrote: >>> >> > Code changes are fine. >>> >> > If two or three errors are present in the configuration file, it >>> >> prints the last error >> >> Ok fine I marked the patch as ready for committer. > > Thanks for review. > > With Regards, > Amit Kapila. > EnterpriseDB: http://www.enterprisedb.com It looks like working as advertised. Great! However I have noticed a few minor issues. 1) validate_conf_option +/* + * Validates configuration parameter and value, by calling check hook functions + * depending on record's vartype. It validates if the parameter + * value given is in range of expected predefined value for that parameter. + * + * freemem - true indicates memory for newval and newextra will be + * freed in this function, false indicates it will be freed + * by caller. + * Return value: + * 1: the value is valid + * 0: the name or value is invalid + */ +int +validate_conf_option(struct config_generic * record, const char *name, + const char *value, GucSource source, int elevel, + bool freemem, void *newval, void **newextra) Is there any reason for the function returns int as it always returns 0 or 1. Maybe returns bool is better? 2) initdb.c + strcpy(tempautobuf, "# Do not edit this file manually! \n"); + autoconflines = pg_strdup(tempautobuf); + strcpy(tempautobuf, "# It will be overwritten by the ALTER SYSTEM command. \n"); + autoconflines = pg_strdup(tempautobuf); Is there any reason to use "tempautobuf" here? I think we can simply change to this: + autoconflines = pg_strdup("# Do not edit this file manually! \n"); + autoconflines = pg_strdup("# It will be overwritten by the ALTER SYSTEM command. \n"); 3) initdb.c It seems the memory allocated for autoconflines and autoconflines by pg_strdup is never freed. (I think there's similar problem with "conflines" as well, though it was not introduced by the patch). Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese: http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (firstname.lastname@example.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers