On Sun, Sep 20, 2009 at 05:59, Abhijit Menon-Sen <a...@toroid.org> wrote: > I think the patch is more or less ready, but I have a few minor > comments: > > First, it needs to be reformatted to not use a space before the opening > parentheses in (some) function calls and definitions. > >> *** a/doc/src/sgml/client-auth.sgml >> --- b/doc/src/sgml/client-auth.sgml >> [...] >> >> + <para>Instead of an <replaceable>CIDR-address</replaceable>, you can >> specify >> + the values <literal>samehost</literal> or >> <literal>samenet</literal>. To >> + match any address on the subnets connected to the local machine, >> specify >> + <literal>samenet</literal>. By specifying >> <literal>samehost</literal>, any >> + addresses present on the network interfaces of local machine will >> match. >> + </para> >> + > > I'd suggest something like the following instead: > > <para>Instead of a <replaceable>CIDR-address</replaceable>, you can > specify <literal>samehost</literal> to match any of the server's own > IP addresses, or <literal>samenet</literal> to match any address in > a subnet that the server belongs to. > >> *** a/src/backend/libpq/hba.c >> --- b/src/backend/libpq/hba.c >> [...] >> >> + else if (addr->sa_family == AF_INET && >> + raddr->addr.ss_family == AF_INET6) >> + { >> + /* >> + * Wrong address family. We allow only one case: if the file >> + * has IPv4 and the port is IPv6, promote the file address to >> + * IPv6 and try to match that way. >> + */ > > How about this instead: > > If we're listening on IPv6 but the file specifies an IPv4 address to > match against, we promote the latter also to an IPv6 address before > trying to match the client's address. > > (The comment is repeated elsewhere.)
That's actually a copy/paste from the code that's in hba.c now. That's not reason not to fix it of course :-) >> + int >> + pg_foreach_ifaddr(PgIfAddrCallback callback, void * cb_data) >> + { >> + #ifdef WIN32 >> + return foreach_ifaddr_win32(callback, cb_data); >> + #else /* !WIN32 */ >> + #ifdef HAVE_GETIFADDRS >> + return foreach_ifaddr_getifaddrs(callback, cb_data); >> + #else /* !HAVE_GETIFADDRS */ >> + return foreach_ifaddr_ifconf(callback, cb_data); >> + #endif >> + #endif /* !WIN32 */ >> + } > > First, writing it this way is less noisy: > > #ifdef WIN32 > return foreach_ifaddr_win32(callback, cb_data); > #elif defined(HAVE_GETIFADDRS) > return foreach_ifaddr_getifaddrs(callback, cb_data); > #else > return foreach_ifaddr_ifconf(callback, cb_data); > #endif > > Second, I can't see that it makes very much difference, but why do it > this way at all? You could just have each of the three #ifdef blocks > define a function named pg_foreach_ifaddr() and be done with it. No > need for a fourth function. That was my thought as well when I looked at the patch. It'd be clearer and I think more in line with what we do at other places. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers