The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: tested, passed Documentation: tested, passed
Hi I applied the patch "v3-0001-Enable-setting-pg_hba.conf-permissions-from-initd.patch" and did some verification with it. The intended feature works overall and I think it is quite useful to support default auth methods for ssl and gss host types. I have also found some minor things in the patch and would like to share as below: > +"# CAUTION: Configuring the system for \"trust\" authentication\n" \ > +"# allows any user who can reach the databse on the route specified\n" \ > +"# to connect as any PostgreSQL user, including the database\n" \ > +"# superuser. If you do not trust all the users who could\n" \ > +"# reach the database on the route specified, use a more restrictive\n" \ > +"# authentication method.\n" Found a typo: should be 'database' instead of 'databse' > * a sort of poor man's grep -v > */ > -#ifndef HAVE_UNIX_SOCKETS > static char ** > filter_lines_with_token(char **lines, const char *token) > { > @@ -461,7 +466,6 @@ filter_lines_with_token(char **lines, const char *token) > > return result; > } > -#endif I see that you have removed "#ifndef HAVE_UNIX_SOCKETS" around the filter_lines_with_token() function definition so it would be always available, which is used to remove the @tokens@ in case user does not specify a default auth method for the new hostssl, hostgss options. I think you should also remove the "#ifndef HAVE_UNIX_SOCKETS" around its declaration as well so both function definition and declaration would make sense. #ifndef HAVE_UNIX_SOCKETS static char **filter_lines_with_token(char **lines, const char *token); #endif Cary Huang ------------- HighGo Software Inc. (Canada) cary.hu...@highgo.ca www.highgo.ca