On 2019-Nov-12, Andres Freund wrote:
> We still have the curious
> proc_exit(0);
> abort(); /* keep the compiler
> quiet */
>
> pattern in WalSndShutdown() - wouldn't the right approach to instead
> proc_exit() with pg_attribute_noreturn()?
proc_exit() is already marked noreturn ... and has been marked as such
since commit eeece9e60984 (2012), which is the same that added abort()
after some proc_exit calls as well as other routines that were already
marked noreturn, such as WalSenderMain(). However, back then we were
using the GCC-specific notation of __attribute__((noreturn)), so perhaps
the reason we kept the abort() (and a few breaks, etc) after proc_exit()
was to satisfy compilers other than GCC.
In modern times, we define pg_attribute_noreturn() like this:
/* GCC, Sunpro and XLC support aligned, packed and noreturn */
#if defined(__GNUC__) || defined(__SUNPRO_C) || defined(__IBMC__)
#define pg_attribute_noreturn() __attribute__((noreturn))
#define HAVE_PG_ATTRIBUTE_NORETURN 1
#else
#define pg_attribute_noreturn()
#endif
I suppose this will cause warnings in compilers other than those, but
I'm not sure if we care. What about MSVC for example?
With the attached patch, everything compiles cleanly in my setup, no
warnings, but then it's GCC.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index bfc629c753..e202ade4b9 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -304,7 +304,6 @@ AuxiliaryProcessMain(int argc, char *argv[])
write_stderr("Try \"%s --help\" for more information.\n",
progname);
proc_exit(1);
- break;
}
}
diff --git a/src/backend/postmaster/startup.c b/src/backend/postmaster/startup.c
index c2250d7d4e..d26868e578 100644
--- a/src/backend/postmaster/startup.c
+++ b/src/backend/postmaster/startup.c
@@ -89,8 +89,8 @@ StartupProcShutdownHandler(SIGNAL_ARGS)
if (in_restore_command)
proc_exit(1);
- else
- shutdown_requested = true;
+
+ shutdown_requested = true;
WakeupRecovery();
errno = save_errno;
diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c
index 77360f1524..45b552cdf2 100644
--- a/src/backend/replication/walreceiver.c
+++ b/src/backend/replication/walreceiver.c
@@ -208,7 +208,6 @@ WalReceiverMain(void)
case WALRCV_STOPPED:
SpinLockRelease(&walrcv->mutex);
proc_exit(1);
- break;
case WALRCV_STARTING:
/* The usual case */
@@ -616,8 +615,8 @@ WalRcvWaitForStartPosition(XLogRecPtr *startpoint, TimeLineID *startpointTLI)
SpinLockRelease(&walrcv->mutex);
if (state == WALRCV_STOPPING)
proc_exit(0);
- else
- elog(FATAL, "unexpected walreceiver state");
+
+ elog(FATAL, "unexpected walreceiver state");
}
walrcv->walRcvState = WALRCV_WAITING;
walrcv->receiveStart = InvalidXLogRecPtr;
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 9c063749b6..99f897cb32 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -337,7 +337,6 @@ WalSndShutdown(void)
whereToSendOutput = DestNone;
proc_exit(0);
- abort(); /* keep the compiler quiet */
}
/*
