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_SSL
        if (port->ssl)
        {
                if (walsender) ..
                else ..
        }
        else
#endif
        ...

-- 
marko



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to