On Jun16, 2011, at 18:46 , Alexey Klyukin wrote: > On Jun 16, 2011, at 6:49 PM, Florian Pflug wrote: >> Hm, wouldn't a test for "context == PGC_POSTMASTER" be more appropriate? > > In such a case the errors caused by command-line arguments won't stop the > postmaster. > PGC_S_FILE seems to handle this correctly. I'm not sure whether it is > appropriate to use > there though.
Ah, yeah, you're right. PGC_S_FILE sounds fine, then. I guess this means you can drop the check for "context == PGC_SIGHUP" though, because surely the source must be PGC_S_DEFAULT or PGC_S_FILE if context == PGC_SIGHUP, right? So the check would become if (source == PGC_S_FILE || source == PGC_S_DEFAULT) where it now says if (context == PGC_SIGHUP || source == PGC_S_DEFAULT) >>>> I see that you basically replaced "goto cleanup..." in both ParseConfigFp() >>>> and ProcessConfigFile() with "++errorcount", and arranged for >>>> ParseConfigFp() >>>> to return false, and for ProcessConfigFile() to skip the GUC updates if >>>> "errorcount > 0". The actual value of errorcount is never inspected. The >>>> value >>>> is also wrong in the case of include files containing more than error, >>>> since >>>> ParseConfigFp() simply increments errorcount by one for each failed >>>> ParseConfigFile() of an included file. >>>> >>>> I thus suggest that you replace "errorcount" with a boolean >>>> "erroroccurred". >>> >>> I can actually pass errorcount down from the ParseConfigFp() to report the >>> total >>> number of errors (w/ the include files) at the end of ProcessConfigFile if >>> there is >>> any interest in having the number of errors reported to a user. If not - >>> I'll change >>> it to boolean. >> >> Nah, just use a boolean, unless you have concrete plans to actually use the >> errorcount >> for something other than test a la "errorcount > 0". > > I just recalled a reason for counting the total number of errors. There is a > condition that > checks that the total number of errors is less than 100 and bails out if it's > more than that > (100 is arbitrary). The reason is to avoid bloating the logs w/ something > totally unrelated > to postgresql.conf. That was suggested by Tom Lane here: > http://archives.postgresql.org/pgsql-hackers/2009-03/msg01142.php Ah, right, I missed that. Guess it'll have to stay a counter, then. Still, I don't think it's worth the effort to make the count correct in case of included files, so I'd say just add a comment explaining that the count isn't totally accurate. best regards, Florian Pflug -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
