There is continued interest in static analyzers (clang, coverity), and
the main problem with those is that they don't know that elog and
ereport with level >= ERROR don't return, leading to lots of false
positives.
I looked in the archive; there were some attempts to fix this some time
ago. One was putting an __attribute__((analyzer_noreturn)) on these
functions, but that was decided to be incorrect (and it would only work
for clang anyway). Another went along the lines of what I'm proposing
here (but before ereport was introduced, if I read it correctly), but
didn't go anywhere.
My proposal with ereport would be to do this:
diff --git i/src/include/utils/elog.h w/src/include/utils/elog.h
--- i/src/include/utils/elog.h
+++ w/src/include/utils/elog.h
@@ -104,7 +104,8 @@
*/
#define ereport_domain(elevel, domain, rest) \
(errstart(elevel, __FILE__, __LINE__, PG_FUNCNAME_MACRO, domain) ? \
- (errfinish rest) : (void) 0)
+ (errfinish rest) : (void) 0), \
+ (elevel >= ERROR ? abort() : (void) 0)
There are no portability problems with that.
With elog, I would do this:
diff --git i/src/include/utils/elog.h w/src/include/utils/elog.h
@@ -196,7 +197,11 @@
* elog(ERROR, "portal \"%s\" not found", stmt->portalname);
*----------
*/
+#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L
+#define elog(elevel, ...) elog_start(__FILE__, __LINE__,
PG_FUNCNAME_MACRO), elog_finish(elevel, __VA_ARGS__), (elevel >= ERROR ?
abort() : (void) 0)
+#else
#define elog elog_start(__FILE__, __LINE__, PG_FUNCNAME_MACRO), elog_finish
+#endif
I think the issue here was that if we support two separate code paths,
we still need to do the actually unreachable /* keep compiler happy */
bits, and that compilers that know about elog not returning would
complain about unreachable code. But I have never seen warnings like
that, so maybe it's a nonissue. But it could be tested if there are
concerns.
Complete patch attached for easier applying and testing.
For clang aficionados: This reduces scan-build warnings from 1222 to 553
for me.
diff --git i/src/include/utils/elog.h w/src/include/utils/elog.h
index 93b141d..b32737e 100644
--- i/src/include/utils/elog.h
+++ w/src/include/utils/elog.h
@@ -104,7 +104,8 @@
*/
#define ereport_domain(elevel, domain, rest) \
(errstart(elevel, __FILE__, __LINE__, PG_FUNCNAME_MACRO, domain) ? \
- (errfinish rest) : (void) 0)
+ (errfinish rest) : (void) 0), \
+ (elevel >= ERROR ? abort() : (void) 0)
#define ereport(elevel, rest) \
ereport_domain(elevel, TEXTDOMAIN, rest)
@@ -196,7 +197,11 @@ extern int getinternalerrposition(void);
* elog(ERROR, "portal \"%s\" not found", stmt->portalname);
*----------
*/
+#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L
+#define elog(elevel, ...) elog_start(__FILE__, __LINE__, PG_FUNCNAME_MACRO), elog_finish(elevel, __VA_ARGS__), (elevel >= ERROR ? abort() : (void) 0)
+#else
#define elog elog_start(__FILE__, __LINE__, PG_FUNCNAME_MACRO), elog_finish
+#endif
extern void elog_start(const char *filename, int lineno, const char *funcname);
extern void
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
