Re: pg_hba.conf: samehost and samenet [REVIEW] - Mailing list pgsql-hackers
From | Magnus Hagander |
---|---|
Subject | Re: pg_hba.conf: samehost and samenet [REVIEW] |
Date | |
Msg-id | 9837222c0909200509v4b8b30e2jf9659342f3b34ef3@mail.gmail.com Whole thread Raw |
In response to | Re: pg_hba.conf: samehost and samenet [REVIEW] (Abhijit Menon-Sen <ams@toroid.org>) |
List | pgsql-hackers |
On Sun, Sep 20, 2009 at 05:59, Abhijit Menon-Sen <ams@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 HaganderMe: http://www.hagander.net/Work: http://www.redpill-linpro.com/
pgsql-hackers by date: