Re: SSL (patch 1) - Mailing list pgsql-patches

From Bear Giles
Subject Re: SSL (patch 1)
Date
Msg-id 200205272133.PAA10832@eris.coyotesong.com
Whole thread Raw
In response to Re: SSL (patch 1)  (Peter Eisentraut <peter_e@gmx.net>)
Responses Re: SSL (patch 1)
List pgsql-patches
> > + extern void secure_close(Port *);
> > + extern ssize_t secure_read(Port *, void *, size_t);
> > + extern ssize_t secure_write(Port *, const void *, size_t);
>
> Should be in a header file.  ssize_t is probably not portable.

I agree, but I don't know enough about PosgreSQL layout policy to
know whether these prototypes should be in their own header file,
in libpq-XXX.h, or some place else.

> Shouldn't secure_close() do all the other closing actions as well?  That's
> how secure_read() etc. appear to work.

It could (should?).  At the moment it forces the *secure* connection
to terminate, but leaves the underlying insecure connection open.
This could be useful if the protocol provided a mechanism to enter
and exit a secure state.  But that probably breaks the 'principal of least
surprise.'

> > Index: postgresql/src/backend/postmaster/be-secure.c
>
> This file seems to be belong under backend/libpq.

See comments above regarding layout.  I put it into postmaster since
the bulk of the code came from postmaster.c.

> > +  * PATCH LEVEL
> > +  *      milestone 1: fix basic coding errors
>
> I don't think we need to record that in a code file.

That's for *you*, and were always meant to be temporary.  I knew there
would be about a dozen concurrent patches in play, and this helps
establish precedence if they don't go in in sequence.

>
> > + #define PING()    elog(DEBUG,"%s, line %d, %s", __FILE__, __LINE__, __func__);
>
> __func__ is definitely not portable.  Nor should there be unconditional
> elog(DEBUG)'s in the code.  Actually, it should be elog(DEBUG[1-5]) now I
> think.

That particular #define is debugging scaffolding.  As for DEBUG[1-5],
I'm using the conventions in the existing software.

> Could also use some libpq_gettext() calls around the strings.

(*cough*)  That code was copied verbatim from the existing software.

Since this is mostly stylistic, I'll put together a patch that runs
against the most recent set of code so that the rest of the patches
will run clean.  Renaming the file would probably best be handled on
your end to avoid unnecessary traffic.

Bear

pgsql-patches by date:

Previous
From: Bear Giles
Date:
Subject: Re: SSL (patch 4)
Next
From: Tom Lane
Date:
Subject: Re: small dblink patch