Re: Feature request: Logging SSL connections

On Fri, Dec 06, 2013 at 11:43:55AM +0100, Dr. Andreas Kunert wrote:
> >>That seems useful.  Do we need more information, like whether a client
> >>certificate was presented, or what ciphers were used?
> >
> >Yes, please show ciphersuite and TLS version too.  Andreas, you can use my
> >recent \conninfo patch as template:
> >
> >   https://github.com/markokr/postgres/commit/7d1b27ac74643abd15007cc4ec0b56ba92b39d90
> >
> >Also, please show the SSL level also for walsender connections.  It's
> >quite important to know whether they are using SSL or not.
> >
> >But I think the 'bits' output is unnecessary, as it's cipher strength
> >is known by ciphersuite.  Perhaps it can be removed from \conninfo too.
> 
> A new patch is attached. I added the ciphersuite and TLS version
> like shown in your template (minus the 'bits' output). I also added
> the SSL information for walsender connections, but due to a missing
> test setup I cannot test that part.
> 
> Anything else missing?

Functionally it's fine now, but I see few style problems:

- "if (port->ssl > 0)" is wrong, ->ssl is pointer.  So use just "if (port->ssl)".

- Deeper indentation would look nicer with braces.

- There are some duplicated message, could you restructure it so that each message exists only once.

Something like this perhaps:

#if USE_SSLif (port->ssl){    if (walsender) ..    else ..}else
#endif...

-- 
marko