[PATCH 1/2 v3] [libpq] rework sigpipe-handling macros - Mailing list pgsql-hackers

Currently, the sigpipe-masking code in libpq is implemented as
a set of macros, which depend on declaring local variables.

This change adds a (private) struct sigpipe_info to contain the
compile-dependent data required for sigpipe masking and restoring.
The caller can then declare a struct sigpipe info explicitly, and
pass this to the subsequent sigpipe-masking code.

This allows us to separate the variable declarations from the code,
and gives the caller more flexibility for controlling the scope of
these variables.

Also, since we don't need to declare variables in the macros, we
can change the code to be implemented as static inlines.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>

---src/interfaces/libpq/fe-secure.c |   88 ++++++++++++++++++++++++---------------1 file changed, 55 insertions(+), 33
deletions(-)

*** a/src/interfaces/libpq/fe-secure.c
--- b/src/interfaces/libpq/fe-secure.c
***************
*** 119,163 **** static long win32_ssl_create_mutex = 0;  /*  * Macros to handle disabling and then restoring the state
ofSIGPIPE handling.
 
-  * Note that DISABLE_SIGPIPE() must appear at the start of a block.  */  #ifndef WIN32 #ifdef ENABLE_THREAD_SAFETY 
! #define DISABLE_SIGPIPE(failaction) \
!     sigset_t    osigmask; \
!     bool        sigpipe_pending; \
!     bool        got_epipe = false; \
! \
!     if (pq_block_sigpipe(&osigmask, &sigpipe_pending) < 0) \
!         failaction 
! #define REMEMBER_EPIPE(cond) \
!     do { \
!         if (cond) \
!             got_epipe = true; \
!     } while (0) 
! #define RESTORE_SIGPIPE() \
!     pq_reset_sigpipe(&osigmask, sigpipe_pending, got_epipe)  #else    /* !ENABLE_THREAD_SAFETY */ 
! #define DISABLE_SIGPIPE(failaction) \
!     pqsigfunc    oldsighandler = pqsignal(SIGPIPE, SIG_IGN) 
! #define REMEMBER_EPIPE(cond) 
! #define RESTORE_SIGPIPE() \
!     pqsignal(SIGPIPE, oldsighandler)  #endif    /* ENABLE_THREAD_SAFETY */ #else    /* WIN32 */ 
! #define DISABLE_SIGPIPE(failaction)
! #define REMEMBER_EPIPE(cond)
! #define RESTORE_SIGPIPE()  #endif    /* WIN32 */ 
--- 119,180 ----  /*  * Macros to handle disabling and then restoring the state of SIGPIPE handling.  */  #ifndef WIN32
#ifdefENABLE_THREAD_SAFETY 
 
! struct sigpipe_info {
!     sigset_t    oldsigmask;
!     bool        sigpipe_pending;
!     bool        got_epipe;
! }; 
! static inline int disable_sigpipe(struct sigpipe_info *info)
! {
!     info->got_epipe = false;
!     return pq_block_sigpipe(&info->oldsigmask, &info->sigpipe_pending) < 0;
! } 
! static inline void remember_epipe(struct sigpipe_info *info, bool cond)
! {
!     if (cond)
!         info->got_epipe = true;
! }
! 
! static inline void restore_sigpipe(struct sigpipe_info *info)
! {
!     pq_reset_sigpipe(&info->oldsigmask, info->sigpipe_pending, info->got_epipe);
! }  #else    /* !ENABLE_THREAD_SAFETY */ 
! struct sigpipe_info {
!     pqsigfunc    oldhandler;
! }; 
! static inline int disable_sigpipe(struct sigpipe_info *info)
! {
!     info->oldhandler = pqsignal(SIGPIPE, SIG_IGN);
!     return 0;
! }
! 
! static inline void remember_epipe(struct sigpipe_info *info, bool cond)
! {
! } 
! static inline void restore_sigpipe(struct sigpipe_info *info)
! {
!     pqsignal(SIGPIPE, info->oldhandler);
! }  #endif    /* ENABLE_THREAD_SAFETY */ #else    /* WIN32 */ 
! struct sigpipe_info { };
! static inline int disable_sigpipe(struct sigpipe_info *info) { return 0; }
! static inline void remember_epipe(struct sigpipe_info *info, bool cond) { }
! static inline void restore_sigpipe(struct sigpipe_info *info) { }  #endif    /* WIN32 */ 
***************
*** 286,294 **** pqsecure_read(PGconn *conn, void *ptr, size_t len)     if (conn->ssl)     {         int
err;         /* SSL_read can write to the socket, so we need to disable SIGPIPE */
 
!         DISABLE_SIGPIPE(return -1);  rloop:         n = SSL_read(conn->ssl, ptr, len);
--- 303,313 ----     if (conn->ssl)     {         int            err;
+         struct sigpipe_info info;          /* SSL_read can write to the socket, so we need to disable SIGPIPE */
!         if (disable_sigpipe(&info))
!             return -1;  rloop:         n = SSL_read(conn->ssl, ptr, len);
***************
*** 315,321 **** rloop:                      if (n == -1)                     {
!                         REMEMBER_EPIPE(SOCK_ERRNO == EPIPE);
printfPQExpBuffer(&conn->errorMessage,                                    libpq_gettext("SSL SYSCALL error: %s\n"),
                       SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
 
--- 334,340 ----                      if (n == -1)                     {
!                         remember_epipe(&info, SOCK_ERRNO == EPIPE);
printfPQExpBuffer(&conn->errorMessage,                                    libpq_gettext("SSL SYSCALL error: %s\n"),
                       SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
 
***************
*** 351,357 **** rloop:                 break;         } 
!         RESTORE_SIGPIPE();     }     else #endif
--- 370,376 ----                 break;         } 
!         restore_sigpipe(&info);     }     else #endif
***************
*** 367,374 **** ssize_t pqsecure_write(PGconn *conn, const void *ptr, size_t len) {     ssize_t        n; 
!     DISABLE_SIGPIPE(return -1);  #ifdef USE_SSL     if (conn->ssl)
--- 386,395 ---- pqsecure_write(PGconn *conn, const void *ptr, size_t len) {     ssize_t        n;
+     struct sigpipe_info info; 
!     if (disable_sigpipe(&info))
!         return -1;  #ifdef USE_SSL     if (conn->ssl)
***************
*** 399,405 **** pqsecure_write(PGconn *conn, const void *ptr, size_t len)                      if (n == -1)
        {
 
!                         REMEMBER_EPIPE(SOCK_ERRNO == EPIPE);
printfPQExpBuffer(&conn->errorMessage,                                    libpq_gettext("SSL SYSCALL error: %s\n"),
                       SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
 
--- 420,426 ----                      if (n == -1)                     {
!                         remember_epipe(&info, SOCK_ERRNO == EPIPE);
printfPQExpBuffer(&conn->errorMessage,                                    libpq_gettext("SSL SYSCALL error: %s\n"),
                       SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
 
***************
*** 438,447 **** pqsecure_write(PGconn *conn, const void *ptr, size_t len) #endif     {         n = send(conn->sock,
ptr,len, 0);
 
!         REMEMBER_EPIPE(n < 0 && SOCK_ERRNO == EPIPE);     } 
!     RESTORE_SIGPIPE();      return n; }
--- 459,468 ---- #endif     {         n = send(conn->sock, ptr, len, 0);
!         remember_epipe(&info, n < 0 && SOCK_ERRNO == EPIPE);     } 
!     restore_sigpipe(&info);      return n; }
***************
*** 1197,1210 **** close_SSL(PGconn *conn) {     if (conn->ssl)     {
!         DISABLE_SIGPIPE((void) 0);         SSL_shutdown(conn->ssl);         SSL_free(conn->ssl);         conn->ssl =
NULL;        pqsecure_destroy();         /* We have to assume we got EPIPE */
 
!         REMEMBER_EPIPE(true);
!         RESTORE_SIGPIPE();     }      if (conn->peer)
--- 1218,1232 ---- {     if (conn->ssl)     {
!         struct sigpipe_info info;
!         disable_sigpipe(&info);         SSL_shutdown(conn->ssl);         SSL_free(conn->ssl);         conn->ssl =
NULL;        pqsecure_destroy();         /* We have to assume we got EPIPE */
 
!         remember_epipe(&info, true);
!         restore_sigpipe(&info);     }      if (conn->peer)