ExceptionalCondition() return type

Started by Peter Eisentrautover 13 years ago2 messages
#1Peter Eisentraut
peter_e@gmx.net

I came across this comment:

/*
* ExceptionalCondition - Handles the failure of an Assert()
*
* Note: this can't actually return, but we declare it as returning int
* because the TrapMacro() macro might get wonky otherwise.
*/

But it seems to me that this can easily be fixed like shown below, which
compiles without warnings. Is there any problem with that?

I noticed that the comment at TrapMacro suggests this usage pattern

#define foo(x) (AssertMacro(x != 0) && bar(x))

but all actual uses of AssertMacro() chain it using the comma operator.
Maybe that had something to do with it in the past, but it doesn't seem
likely either.

diff --git i/src/backend/utils/error/assert.c w/src/backend/utils/error/assert.c
index 34909f7..1b4d14d 100644
--- i/src/backend/utils/error/assert.c
+++ w/src/backend/utils/error/assert.c
@@ -25,7 +25,7 @@
  * Note: this can't actually return, but we declare it as returning int
  * because the TrapMacro() macro might get wonky otherwise.
  */
-int
+void
 ExceptionalCondition(const char *conditionName,
 					 const char *errorType,
 					 const char *fileName,
@@ -55,6 +55,4 @@ ExceptionalCondition(const char *conditionName,
 #endif
 	abort();
-
-	return 0;
 }
diff --git i/src/include/postgres.h w/src/include/postgres.h
index 60a2bdb..a3181d6 100644
--- i/src/include/postgres.h
+++ w/src/include/postgres.h
@@ -662,7 +662,7 @@ extern PGDLLIMPORT bool assert_enabled;
 #define TrapMacro(condition, errorType) \
 	((bool) ((! assert_enabled) || ! (condition) || \
 			 (ExceptionalCondition(CppAsString(condition), (errorType), \
-								   __FILE__, __LINE__))))
+								   __FILE__, __LINE__), 0)))

#ifndef USE_ASSERT_CHECKING
#define Assert(condition)
@@ -683,8 +683,8 @@ extern PGDLLIMPORT bool assert_enabled;
Trap(!(condition), "BadState")
#endif /* USE_ASSERT_CHECKING */

-extern int ExceptionalCondition(const char *conditionName,
+extern void ExceptionalCondition(const char *conditionName,
 					 const char *errorType,
-					 const char *fileName, int lineNumber);
+					 const char *fileName, int lineNumber) __attribute__((noreturn));

#endif /* POSTGRES_H */

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#1)
Re: ExceptionalCondition() return type

Peter Eisentraut <peter_e@gmx.net> writes:

I came across this comment:
/*
* ExceptionalCondition - Handles the failure of an Assert()
*
* Note: this can't actually return, but we declare it as returning int
* because the TrapMacro() macro might get wonky otherwise.
*/

But it seems to me that this can easily be fixed like shown below, which
compiles without warnings. Is there any problem with that?

Um ... you did not fix the comment.

I noticed that the comment at TrapMacro suggests this usage pattern
#define foo(x) (AssertMacro(x != 0) && bar(x))
but all actual uses of AssertMacro() chain it using the comma operator.

Should probably adjust that comment to suggest commas, too.

regards, tom lane