[PATCH 1/2] [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(-)

diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c
index ee0a91e..13c97ac 100644
--- a/src/interfaces/libpq/fe-secure.c
+++ b/src/interfaces/libpq/fe-secure.c
@@ -119,45 +119,62 @@ static long win32_ssl_create_mutex = 0;/* * Macros to handle disabling and then restoring the
stateof SIGPIPE 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
+struct sigpipe_info {
+    sigset_t    oldsigmask;
+    bool        sigpipe_pending;
+    bool        got_epipe;
+};
-#define REMEMBER_EPIPE(cond) \
-    do { \
-        if (cond) \
-            got_epipe = true; \
-    } while (0)
+static inline int disable_sigpipe(struct sigpipe_info *info)
+{
+    info->got_epipe = false;
+    return pq_block_sigpipe(&info->oldsigmask, &info->sigpipe_pending) < 0;
+}
-#define RESTORE_SIGPIPE() \
-    pq_reset_sigpipe(&osigmask, sigpipe_pending, got_epipe)
+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 */
-#define DISABLE_SIGPIPE(failaction) \
-    pqsigfunc    oldsighandler = pqsignal(SIGPIPE, SIG_IGN)
+struct sigpipe_info {
+    pqsigfunc    oldhandler;
+};
-#define REMEMBER_EPIPE(cond)
+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)
+{
+}
-#define RESTORE_SIGPIPE() \
-    pqsignal(SIGPIPE, oldsighandler)
+static inline void restore_sigpipe(struct sigpipe_info *info)
+{
+    pqsignal(SIGPIPE, info->oldhandler);
+}#endif    /* ENABLE_THREAD_SAFETY */#else    /* WIN32 */
-#define DISABLE_SIGPIPE(failaction)
-#define REMEMBER_EPIPE(cond)
-#define RESTORE_SIGPIPE()
+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,9 +303,11 @@ pqsecure_read(PGconn *conn, void *ptr, size_t len)    if (conn->ssl)    {        int
err;
+        struct sigpipe_info info;        /* SSL_read can write to the socket, so we need to disable SIGPIPE */
-        DISABLE_SIGPIPE(return -1);
+        if (disable_sigpipe(&info))
+            return -1;rloop:        n = SSL_read(conn->ssl, ptr, len);
@@ -315,7 +334,7 @@ rloop:                    if (n == -1)                    {
-                        REMEMBER_EPIPE(SOCK_ERRNO == EPIPE);
+                        remember_epipe(&info, SOCK_ERRNO == EPIPE);
printfPQExpBuffer(&conn->errorMessage,                                   libpq_gettext("SSL SYSCALL error: %s\n"),
                     SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
 
@@ -351,7 +370,7 @@ rloop:                break;        }
-        RESTORE_SIGPIPE();
+        restore_sigpipe(&info);    }    else#endif
@@ -367,8 +386,10 @@ ssize_tpqsecure_write(PGconn *conn, const void *ptr, size_t len){    ssize_t        n;
+    struct sigpipe_info info;
-    DISABLE_SIGPIPE(return -1);
+    if (disable_sigpipe(&info))
+        return -1;#ifdef USE_SSL    if (conn->ssl)
@@ -399,7 +420,7 @@ pqsecure_write(PGconn *conn, const void *ptr, size_t len)                    if (n == -1)
        {
 
-                        REMEMBER_EPIPE(SOCK_ERRNO == EPIPE);
+                        remember_epipe(&info, SOCK_ERRNO == EPIPE);
printfPQExpBuffer(&conn->errorMessage,                                   libpq_gettext("SSL SYSCALL error: %s\n"),
                     SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
 
@@ -438,10 +459,10 @@ 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);
+        remember_epipe(&info, n < 0 && SOCK_ERRNO == EPIPE);    }
-    RESTORE_SIGPIPE();
+    restore_sigpipe(&info);    return n;}
@@ -1197,14 +1218,15 @@ close_SSL(PGconn *conn){    if (conn->ssl)    {
-        DISABLE_SIGPIPE((void) 0);
+        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(true);
-        RESTORE_SIGPIPE();
+        remember_epipe(&info, true);
+        restore_sigpipe(&info);    }    if (conn->peer)