Code review for recent libpq changes. Be more careful about error
handling in SIGPIPE processing; avoid unnecessary pollution of application link-symbol namespace; spell 'pointer to function' in the conventional way.
This commit is contained in:
Tom Lane
parent
9b711e766e
commit
5d7a555d0f
|
|
@ -8,7 +8,7 @@
|
|||
*
|
||||
*
|
||||
* IDENTIFICATION
|
||||
* $PostgreSQL: pgsql/src/interfaces/libpq/fe-connect.c,v 1.291 2004/12/02 15:32:54 momjian Exp $
|
||||
* $PostgreSQL: pgsql/src/interfaces/libpq/fe-connect.c,v 1.292 2004/12/02 23:20:19 tgl Exp $
|
||||
*
|
||||
*-------------------------------------------------------------------------
|
||||
*/
|
||||
|
|
@ -202,6 +202,11 @@ static int parseServiceInfo(PQconninfoOption *options,
|
|||
static char *pwdfMatchesString(char *buf, char *token);
|
||||
static char *PasswordFromFile(char *hostname, char *port, char *dbname,
|
||||
char *username);
|
||||
static void default_threadlock(int acquire);
|
||||
|
||||
|
||||
/* global variable because fe-auth.c needs to access it */
|
||||
pgthreadlock_t pg_g_threadlock = default_threadlock;
|
||||
|
||||
|
||||
/*
|
||||
|
|
@ -3276,16 +3281,6 @@ PasswordFromFile(char *hostname, char *port, char *dbname, char *username)
|
|||
* if they are not required.
|
||||
*/
|
||||
|
||||
void
|
||||
PQinitSSL(int do_init)
|
||||
{
|
||||
#ifdef USE_SSL
|
||||
pq_initssllib = do_init;
|
||||
#endif
|
||||
}
|
||||
|
||||
static pgthreadlock_t default_threadlock;
|
||||
|
||||
static void
|
||||
default_threadlock(int acquire)
|
||||
{
|
||||
|
|
@ -3313,17 +3308,15 @@ default_threadlock(int acquire)
|
|||
#endif
|
||||
}
|
||||
|
||||
pgthreadlock_t *g_threadlock = default_threadlock;
|
||||
|
||||
pgthreadlock_t *
|
||||
PQregisterThreadLock(pgthreadlock_t *newhandler)
|
||||
pgthreadlock_t
|
||||
PQregisterThreadLock(pgthreadlock_t newhandler)
|
||||
{
|
||||
pgthreadlock_t *prev;
|
||||
pgthreadlock_t prev = pg_g_threadlock;
|
||||
|
||||
prev = g_threadlock;
|
||||
if (newhandler)
|
||||
g_threadlock = newhandler;
|
||||
pg_g_threadlock = newhandler;
|
||||
else
|
||||
g_threadlock = default_threadlock;
|
||||
pg_g_threadlock = default_threadlock;
|
||||
|
||||
return prev;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -10,7 +10,7 @@
|
|||
* didn't really belong there.
|
||||
*
|
||||
* IDENTIFICATION
|
||||
* $PostgreSQL: pgsql/src/interfaces/libpq/fe-print.c,v 1.56 2004/12/02 15:32:54 momjian Exp $
|
||||
* $PostgreSQL: pgsql/src/interfaces/libpq/fe-print.c,v 1.57 2004/12/02 23:20:20 tgl Exp $
|
||||
*
|
||||
*-------------------------------------------------------------------------
|
||||
*/
|
||||
|
|
@ -193,8 +193,8 @@ PQprint(FILE *fout,
|
|||
{
|
||||
usePipe = 1;
|
||||
#ifdef ENABLE_THREAD_SAFETY
|
||||
pq_block_sigpipe(&osigset, &sigpipe_pending);
|
||||
sigpipe_masked = true;
|
||||
if (pq_block_sigpipe(&osigset, &sigpipe_pending) == 0)
|
||||
sigpipe_masked = true;
|
||||
#else
|
||||
#ifndef WIN32
|
||||
oldsigpipehandler = pqsignal(SIGPIPE, SIG_IGN);
|
||||
|
|
@ -316,8 +316,9 @@ PQprint(FILE *fout,
|
|||
pclose(fout);
|
||||
#endif
|
||||
#ifdef ENABLE_THREAD_SAFETY
|
||||
/* we can't easily verify if EPIPE occurred, so say it did */
|
||||
if (sigpipe_masked)
|
||||
pq_reset_sigpipe(&osigset, sigpipe_pending);
|
||||
pq_reset_sigpipe(&osigset, sigpipe_pending, true);
|
||||
#else
|
||||
#ifndef WIN32
|
||||
pqsignal(SIGPIPE, oldsigpipehandler);
|
||||
|
|
|
|||
|
|
@ -11,7 +11,7 @@
|
|||
*
|
||||
*
|
||||
* IDENTIFICATION
|
||||
* $PostgreSQL: pgsql/src/interfaces/libpq/fe-secure.c,v 1.58 2004/12/02 15:32:54 momjian Exp $
|
||||
* $PostgreSQL: pgsql/src/interfaces/libpq/fe-secure.c,v 1.59 2004/12/02 23:20:21 tgl Exp $
|
||||
*
|
||||
* NOTES
|
||||
* [ Most of these notes are wrong/obsolete, but perhaps not all ]
|
||||
|
|
@ -147,7 +147,7 @@ static void SSLerrfree(char *buf);
|
|||
#endif
|
||||
|
||||
#ifdef USE_SSL
|
||||
bool pq_initssllib = true;
|
||||
static bool pq_initssllib = true;
|
||||
|
||||
static SSL_CTX *SSL_context = NULL;
|
||||
#endif
|
||||
|
|
@ -212,6 +212,19 @@ KWbuHn491xNO25CQWMtem80uKw+pTnisBRF/454n1Jnhub144YRBoN8CAQI=\n\
|
|||
/* Procedures common to all secure sessions */
|
||||
/* ------------------------------------------------------------ */
|
||||
|
||||
|
||||
/*
|
||||
* Exported (but as yet undocumented) function to allow application to
|
||||
* tell us it's already initialized OpenSSL.
|
||||
*/
|
||||
void
|
||||
PQinitSSL(int do_init)
|
||||
{
|
||||
#ifdef USE_SSL
|
||||
pq_initssllib = do_init;
|
||||
#endif
|
||||
}
|
||||
|
||||
/*
|
||||
* Initialize global context
|
||||
*/
|
||||
|
|
@ -377,8 +390,10 @@ pqsecure_write(PGconn *conn, const void *ptr, size_t len)
|
|||
#ifdef ENABLE_THREAD_SAFETY
|
||||
sigset_t osigmask;
|
||||
bool sigpipe_pending;
|
||||
bool got_epipe = false;
|
||||
|
||||
pq_block_sigpipe(&osigmask, &sigpipe_pending);
|
||||
if (pq_block_sigpipe(&osigmask, &sigpipe_pending) < 0)
|
||||
return -1;
|
||||
#else
|
||||
#ifndef WIN32
|
||||
pqsigfunc oldsighandler = pqsignal(SIGPIPE, SIG_IGN);
|
||||
|
|
@ -413,9 +428,13 @@ pqsecure_write(PGconn *conn, const void *ptr, size_t len)
|
|||
char sebuf[256];
|
||||
|
||||
if (n == -1)
|
||||
{
|
||||
if (SOCK_ERRNO == EPIPE)
|
||||
got_epipe = true;
|
||||
printfPQExpBuffer(&conn->errorMessage,
|
||||
libpq_gettext("SSL SYSCALL error: %s\n"),
|
||||
SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
|
||||
}
|
||||
else
|
||||
{
|
||||
printfPQExpBuffer(&conn->errorMessage,
|
||||
|
|
@ -448,15 +467,16 @@ pqsecure_write(PGconn *conn, const void *ptr, size_t len)
|
|||
}
|
||||
else
|
||||
#endif
|
||||
{
|
||||
n = send(conn->sock, ptr, len, 0);
|
||||
/*
|
||||
* Possible optimization: if sigpending() turns out to be an
|
||||
* expensive operation, we can set sigpipe_pending = 'true'
|
||||
* here if errno != EPIPE, avoiding a sigpending call.
|
||||
*/
|
||||
#ifdef ENABLE_THREAD_SAFETY
|
||||
if (n < 0 && SOCK_ERRNO == EPIPE)
|
||||
got_epipe = true;
|
||||
#endif
|
||||
}
|
||||
|
||||
#ifdef ENABLE_THREAD_SAFETY
|
||||
pq_reset_sigpipe(&osigmask, sigpipe_pending);
|
||||
pq_reset_sigpipe(&osigmask, sigpipe_pending, got_epipe);
|
||||
#else
|
||||
#ifndef WIN32
|
||||
pqsignal(SIGPIPE, oldsighandler);
|
||||
|
|
@ -1228,13 +1248,14 @@ pq_block_sigpipe(sigset_t *osigset, bool *sigpipe_pending)
|
|||
{
|
||||
sigset_t sigpipe_sigset;
|
||||
sigset_t sigset;
|
||||
int ret;
|
||||
|
||||
sigemptyset(&sigpipe_sigset);
|
||||
sigaddset(&sigpipe_sigset, SIGPIPE);
|
||||
|
||||
/* Block SIGPIPE and save previous mask for later reset */
|
||||
ret = pthread_sigmask(SIG_BLOCK, &sigpipe_sigset, osigset);
|
||||
SOCK_ERRNO_SET(pthread_sigmask(SIG_BLOCK, &sigpipe_sigset, osigset));
|
||||
if (SOCK_ERRNO)
|
||||
return -1;
|
||||
|
||||
/* We can have a pending SIGPIPE only if it was blocked before */
|
||||
if (sigismember(osigset, SIGPIPE))
|
||||
|
|
@ -1251,44 +1272,52 @@ pq_block_sigpipe(sigset_t *osigset, bool *sigpipe_pending)
|
|||
else
|
||||
*sigpipe_pending = false;
|
||||
|
||||
return ret;
|
||||
return 0;
|
||||
}
|
||||
|
||||
/*
|
||||
* Discard any pending SIGPIPE and reset the signal mask.
|
||||
* We might be discarding a blocked SIGPIPE that we didn't generate,
|
||||
* but we document that you can't keep blocked SIGPIPE calls across
|
||||
* libpq function calls.
|
||||
*
|
||||
* Note: we are effectively assuming here that the C library doesn't queue
|
||||
* up multiple SIGPIPE events. If it did, then we'd accidentally leave
|
||||
* ours in the queue when an event was already pending and we got another.
|
||||
* As long as it doesn't queue multiple events, we're OK because the caller
|
||||
* can't tell the difference.
|
||||
*
|
||||
* The caller should say got_epipe = FALSE if it is certain that it
|
||||
* didn't get an EPIPE error; in that case we'll skip the clear operation
|
||||
* and things are definitely OK, queuing or no. If it got one or might have
|
||||
* gotten one, pass got_epipe = TRUE.
|
||||
*
|
||||
* We do not want this to change errno, since if it did that could lose
|
||||
* the error code from a preceding send(). We essentially assume that if
|
||||
* we were able to do pq_block_sigpipe(), this can't fail.
|
||||
*/
|
||||
int
|
||||
pq_reset_sigpipe(sigset_t *osigset, bool sigpipe_pending)
|
||||
void
|
||||
pq_reset_sigpipe(sigset_t *osigset, bool sigpipe_pending, bool got_epipe)
|
||||
{
|
||||
int save_errno = SOCK_ERRNO;
|
||||
int signo;
|
||||
sigset_t sigset;
|
||||
|
||||
/* Clear SIGPIPE only if none was pending */
|
||||
if (!sigpipe_pending)
|
||||
if (got_epipe && !sigpipe_pending)
|
||||
{
|
||||
if (sigpending(&sigset) != 0)
|
||||
return -1;
|
||||
|
||||
/*
|
||||
* Discard pending and blocked SIGPIPE
|
||||
*/
|
||||
if (sigismember(&sigset, SIGPIPE))
|
||||
if (sigpending(&sigset) == 0 &&
|
||||
sigismember(&sigset, SIGPIPE))
|
||||
{
|
||||
sigset_t sigpipe_sigset;
|
||||
|
||||
sigemptyset(&sigpipe_sigset);
|
||||
sigaddset(&sigpipe_sigset, SIGPIPE);
|
||||
|
||||
|
||||
sigwait(&sigpipe_sigset, &signo);
|
||||
if (signo != SIGPIPE)
|
||||
return -1;
|
||||
}
|
||||
}
|
||||
|
||||
/* Restore saved block mask */
|
||||
return pthread_sigmask(SIG_SETMASK, osigset, NULL);
|
||||
pthread_sigmask(SIG_SETMASK, osigset, NULL);
|
||||
|
||||
SOCK_ERRNO_SET(save_errno);
|
||||
}
|
||||
#endif
|
||||
|
|
|
|||
|
|
@ -7,7 +7,7 @@
|
|||
* Portions Copyright (c) 1996-2004, PostgreSQL Global Development Group
|
||||
* Portions Copyright (c) 1994, Regents of the University of California
|
||||
*
|
||||
* $PostgreSQL: pgsql/src/interfaces/libpq/libpq-fe.h,v 1.114 2004/12/02 15:32:54 momjian Exp $
|
||||
* $PostgreSQL: pgsql/src/interfaces/libpq/libpq-fe.h,v 1.115 2004/12/02 23:20:21 tgl Exp $
|
||||
*
|
||||
*-------------------------------------------------------------------------
|
||||
*/
|
||||
|
|
@ -279,6 +279,9 @@ extern SSL *PQgetssl(PGconn *conn);
|
|||
extern void *PQgetssl(PGconn *conn);
|
||||
#endif
|
||||
|
||||
/* Tell libpq whether it needs to initialize OpenSSL */
|
||||
extern void PQinitSSL(int do_init);
|
||||
|
||||
/* Set verbosity for PQerrorMessage and PQresultErrorMessage */
|
||||
extern PGVerbosity PQsetErrorVerbosity(PGconn *conn, PGVerbosity verbosity);
|
||||
|
||||
|
|
@ -301,11 +304,9 @@ extern PQnoticeProcessor PQsetNoticeProcessor(PGconn *conn,
|
|||
* Only required for multithreaded apps that use kerberos
|
||||
* both within their app and for postgresql connections.
|
||||
*/
|
||||
typedef void (pgthreadlock_t) (int acquire);
|
||||
typedef void (*pgthreadlock_t) (int acquire);
|
||||
|
||||
extern pgthreadlock_t *PQregisterThreadLock(pgthreadlock_t *newhandler);
|
||||
|
||||
extern void PQinitSSL(int do_init);
|
||||
extern pgthreadlock_t PQregisterThreadLock(pgthreadlock_t newhandler);
|
||||
|
||||
/* === in fe-exec.c === */
|
||||
|
||||
|
|
@ -495,8 +496,6 @@ extern int PQdsplen(const unsigned char *s, int encoding);
|
|||
/* Get encoding id from environment variable PGCLIENTENCODING */
|
||||
extern int PQenv2encoding(void);
|
||||
|
||||
/* === in fe-secure.c === */
|
||||
|
||||
#ifdef __cplusplus
|
||||
}
|
||||
#endif
|
||||
|
|
|
|||
|
|
@ -12,7 +12,7 @@
|
|||
* Portions Copyright (c) 1996-2004, PostgreSQL Global Development Group
|
||||
* Portions Copyright (c) 1994, Regents of the University of California
|
||||
*
|
||||
* $PostgreSQL: pgsql/src/interfaces/libpq/libpq-int.h,v 1.97 2004/12/02 15:32:54 momjian Exp $
|
||||
* $PostgreSQL: pgsql/src/interfaces/libpq/libpq-int.h,v 1.98 2004/12/02 23:20:21 tgl Exp $
|
||||
*
|
||||
*-------------------------------------------------------------------------
|
||||
*/
|
||||
|
|
@ -379,13 +379,13 @@ extern int pqPacketSend(PGconn *conn, char pack_type,
|
|||
const void *buf, size_t buf_len);
|
||||
|
||||
#ifdef ENABLE_THREAD_SAFETY
|
||||
extern pgthreadlock_t *g_threadlock;
|
||||
extern pgthreadlock_t pg_g_threadlock;
|
||||
|
||||
#define pglock_thread() g_threadlock(true);
|
||||
#define pgunlock_thread() g_threadlock(false);
|
||||
#define pglock_thread() pg_g_threadlock(true)
|
||||
#define pgunlock_thread() pg_g_threadlock(false)
|
||||
#else
|
||||
#define pglock_thread() ((void)0)
|
||||
#define pgunlock_thread() ((void)0)
|
||||
#define pglock_thread() ((void) 0)
|
||||
#define pgunlock_thread() ((void) 0)
|
||||
#endif
|
||||
|
||||
|
||||
|
|
@ -476,13 +476,10 @@ extern void pqsecure_close(PGconn *);
|
|||
extern ssize_t pqsecure_read(PGconn *, void *ptr, size_t len);
|
||||
extern ssize_t pqsecure_write(PGconn *, const void *ptr, size_t len);
|
||||
|
||||
#ifdef USE_SSL
|
||||
extern bool pq_initssllib;
|
||||
#endif
|
||||
|
||||
#ifdef ENABLE_THREAD_SAFETY
|
||||
int pq_block_sigpipe(sigset_t *osigset, bool *sigpipe_pending);
|
||||
int pq_reset_sigpipe(sigset_t *osigset, bool sigpipe_pending);
|
||||
extern int pq_block_sigpipe(sigset_t *osigset, bool *sigpipe_pending);
|
||||
extern void pq_reset_sigpipe(sigset_t *osigset, bool sigpipe_pending,
|
||||
bool got_epipe);
|
||||
#endif
|
||||
|
||||
/*
|
||||
|
|
|
|||
Loading…
Reference in New Issue