Improve the internal implementation of ereport().

Change all the auxiliary error-reporting routines to return void,
now that we no longer need to pretend they are passing something
useful to errfinish().  While this probably doesn't save anything
significant at the machine-code level, it allows detection of some
additional types of mistakes.

Pass the error location details (__FILE__, __LINE__, PG_FUNCNAME_MACRO)
to errfinish not errstart.  This shaves a few cycles off the case where
errstart decides we're not going to emit anything.

Re-implement elog() as a trivial wrapper around ereport(), removing
the separate support infrastructure it used to have.  Aside from
getting rid of some now-surplus code, this means that elog() now
really does have exactly the same semantics as ereport(), in particular
that it can skip evaluation work if the message is not to be emitted.

Andres Freund and Tom Lane

Discussion: https://postgr.es/m/CA+fd4k6N8EjNvZpM8nme+y+05mz-SM8Z_BgkixzkA34R+ej0Kw@mail.gmail.com

1 files changed, 49 insertions, 185 deletions

diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index de705c86fa1..6ac2152ddfe 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -223,17 +223,15 @@ err_gettext(const char *str)
 /*
  * errstart --- begin an error-reporting cycle
  *
- * Create a stack entry and store the given parameters in it.  Subsequently,
- * errmsg() and perhaps other routines will be called to further populate
- * the stack entry.  Finally, errfinish() will be called to actually process
- * the error report.
+ * Create and initialize error stack entry.  Subsequently, errmsg() and
+ * perhaps other routines will be called to further populate the stack entry.
+ * Finally, errfinish() will be called to actually process the error report.
  *
  * Returns true in normal case.  Returns false to short-circuit the error
  * report (if it's a warning or lower and not to be reported anywhere).
  */
 bool
-errstart(int elevel, const char *filename, int lineno,
-		 const char *funcname, const char *domain)
+errstart(int elevel, const char *domain)
 {
 	ErrorData  *edata;
 	bool		output_to_server;
@@ -321,8 +319,7 @@ errstart(int elevel, const char *filename, int lineno,
 	if (ErrorContext == NULL)
 	{
 		/* Oops, hard crash time; very little we can do safely here */
-		write_stderr("error occurred at %s:%d before error message processing is available\n",
-					 filename ? filename : "(unknown file)", lineno);
+		write_stderr("error occurred before error message processing is available\n");
 		exit(2);
 	}
 
@@ -368,18 +365,6 @@ errstart(int elevel, const char *filename, int lineno,
 	edata->elevel = elevel;
 	edata->output_to_server = output_to_server;
 	edata->output_to_client = output_to_client;
-	if (filename)
-	{
-		const char *slash;
-
-		/* keep only base name, useful especially for vpath builds */
-		slash = strrchr(filename, '/');
-		if (slash)
-			filename = slash + 1;
-	}
-	edata->filename = filename;
-	edata->lineno = lineno;
-	edata->funcname = funcname;
 	/* the default text domain is the backend's */
 	edata->domain = domain ? domain : PG_TEXTDOMAIN("postgres");
 	/* initialize context_domain the same way (see set_errcontext_domain()) */
@@ -434,11 +419,11 @@ matches_backtrace_functions(const char *funcname)
  *
  * Produce the appropriate error report(s) and pop the error stack.
  *
- * If elevel is ERROR or worse, control does not return to the caller.
- * See elog.h for the error level definitions.
+ * If elevel, as passed to errstart(), is ERROR or worse, control does not
+ * return to the caller.  See elog.h for the error level definitions.
  */
 void
-errfinish(int dummy,...)
+errfinish(const char *filename, int lineno, const char *funcname)
 {
 	ErrorData  *edata = &errordata[errordata_stack_depth];
 	int			elevel;
@@ -447,6 +432,22 @@ errfinish(int dummy,...)
 
 	recursion_depth++;
 	CHECK_STACK_DEPTH();
+
+	/* Save the last few bits of error state into the stack entry */
+	if (filename)
+	{
+		const char *slash;
+
+		/* keep only base name, useful especially for vpath builds */
+		slash = strrchr(filename, '/');
+		if (slash)
+			filename = slash + 1;
+	}
+
+	edata->filename = filename;
+	edata->lineno = lineno;
+	edata->funcname = funcname;
+
 	elevel = edata->elevel;
 
 	/*
@@ -605,7 +606,7 @@ errfinish(int dummy,...)
  *
  * The code is expected to be represented as per MAKE_SQLSTATE().
  */
-int
+void
 errcode(int sqlerrcode)
 {
 	ErrorData  *edata = &errordata[errordata_stack_depth];
@@ -614,8 +615,6 @@ errcode(int sqlerrcode)
 	CHECK_STACK_DEPTH();
 
 	edata->sqlerrcode = sqlerrcode;
-
-	return 0;					/* return value does not matter */
 }
 
 
@@ -628,7 +627,7 @@ errcode(int sqlerrcode)
  * NOTE: the primary error message string should generally include %m
  * when this is used.
  */
-int
+void
 errcode_for_file_access(void)
 {
 	ErrorData  *edata = &errordata[errordata_stack_depth];
@@ -686,8 +685,6 @@ errcode_for_file_access(void)
 			edata->sqlerrcode = ERRCODE_INTERNAL_ERROR;
 			break;
 	}
-
-	return 0;					/* return value does not matter */
 }
 
 /*
@@ -699,7 +696,7 @@ errcode_for_file_access(void)
  * NOTE: the primary error message string should generally include %m
  * when this is used.
  */
-int
+void
 errcode_for_socket_access(void)
 {
 	ErrorData  *edata = &errordata[errordata_stack_depth];
@@ -722,8 +719,6 @@ errcode_for_socket_access(void)
 			edata->sqlerrcode = ERRCODE_INTERNAL_ERROR;
 			break;
 	}
-
-	return 0;					/* return value does not matter */
 }
 
 
@@ -819,7 +814,7 @@ errcode_for_socket_access(void)
  * Note: no newline is needed at the end of the fmt string, since
  * ereport will provide one for the output methods that need it.
  */
-int
+void
 errmsg(const char *fmt,...)
 {
 	ErrorData  *edata = &errordata[errordata_stack_depth];
@@ -834,14 +829,13 @@ errmsg(const char *fmt,...)
 
 	MemoryContextSwitchTo(oldcontext);
 	recursion_depth--;
-	return 0;					/* return value does not matter */
 }
 
 /*
  * Add a backtrace to the containing ereport() call.  This is intended to be
  * added temporarily during debugging.
  */
-int
+void
 errbacktrace(void)
 {
 	ErrorData   *edata = &errordata[errordata_stack_depth];
@@ -855,8 +849,6 @@ errbacktrace(void)
 
 	MemoryContextSwitchTo(oldcontext);
 	recursion_depth--;
-
-	return 0;
 }
 
 /*
@@ -906,7 +898,7 @@ set_backtrace(ErrorData *edata, int num_skip)
  * the message because the translation would fail and result in infinite
  * error recursion.
  */
-int
+void
 errmsg_internal(const char *fmt,...)
 {
 	ErrorData  *edata = &errordata[errordata_stack_depth];
@@ -921,7 +913,6 @@ errmsg_internal(const char *fmt,...)
 
 	MemoryContextSwitchTo(oldcontext);
 	recursion_depth--;
-	return 0;					/* return value does not matter */
 }
 
 
@@ -929,7 +920,7 @@ errmsg_internal(const char *fmt,...)
  * errmsg_plural --- add a primary error message text to the current error,
  * with support for pluralization of the message text
  */
-int
+void
 errmsg_plural(const char *fmt_singular, const char *fmt_plural,
 			  unsigned long n,...)
 {
@@ -945,14 +936,13 @@ errmsg_plural(const char *fmt_singular, const char *fmt_plural,
 
 	MemoryContextSwitchTo(oldcontext);
 	recursion_depth--;
-	return 0;					/* return value does not matter */
 }
 
 
 /*
  * errdetail --- add a detail error message text to the current error
  */
-int
+void
 errdetail(const char *fmt,...)
 {
 	ErrorData  *edata = &errordata[errordata_stack_depth];
@@ -966,7 +956,6 @@ errdetail(const char *fmt,...)
 
 	MemoryContextSwitchTo(oldcontext);
 	recursion_depth--;
-	return 0;					/* return value does not matter */
 }
 
 
@@ -979,7 +968,7 @@ errdetail(const char *fmt,...)
  * messages that seem not worth translating for one reason or another
  * (typically, that they don't seem to be useful to average users).
  */
-int
+void
 errdetail_internal(const char *fmt,...)
 {
 	ErrorData  *edata = &errordata[errordata_stack_depth];
@@ -993,14 +982,13 @@ errdetail_internal(const char *fmt,...)
 
 	MemoryContextSwitchTo(oldcontext);
 	recursion_depth--;
-	return 0;					/* return value does not matter */
 }
 
 
 /*
  * errdetail_log --- add a detail_log error message text to the current error
  */
-int
+void
 errdetail_log(const char *fmt,...)
 {
 	ErrorData  *edata = &errordata[errordata_stack_depth];
@@ -1014,14 +1002,13 @@ errdetail_log(const char *fmt,...)
 
 	MemoryContextSwitchTo(oldcontext);
 	recursion_depth--;
-	return 0;					/* return value does not matter */
 }
 
 /*
  * errdetail_log_plural --- add a detail_log error message text to the current error
  * with support for pluralization of the message text
  */
-int
+void
 errdetail_log_plural(const char *fmt_singular, const char *fmt_plural,
 					 unsigned long n,...)
 {
@@ -1036,7 +1023,6 @@ errdetail_log_plural(const char *fmt_singular, const char *fmt_plural,
 
 	MemoryContextSwitchTo(oldcontext);
 	recursion_depth--;
-	return 0;					/* return value does not matter */
 }
 
 
@@ -1044,7 +1030,7 @@ errdetail_log_plural(const char *fmt_singular, const char *fmt_plural,
  * errdetail_plural --- add a detail error message text to the current error,
  * with support for pluralization of the message text
  */
-int
+void
 errdetail_plural(const char *fmt_singular, const char *fmt_plural,
 				 unsigned long n,...)
 {
@@ -1059,14 +1045,13 @@ errdetail_plural(const char *fmt_singular, const char *fmt_plural,
 
 	MemoryContextSwitchTo(oldcontext);
 	recursion_depth--;
-	return 0;					/* return value does not matter */
 }
 
 
 /*
  * errhint --- add a hint error message text to the current error
  */
-int
+void
 errhint(const char *fmt,...)
 {
 	ErrorData  *edata = &errordata[errordata_stack_depth];
@@ -1080,7 +1065,6 @@ errhint(const char *fmt,...)
 
 	MemoryContextSwitchTo(oldcontext);
 	recursion_depth--;
-	return 0;					/* return value does not matter */
 }
 
 
@@ -1091,7 +1075,7 @@ errhint(const char *fmt,...)
  * context information.  We assume earlier calls represent more-closely-nested
  * states.
  */
-int
+void
 errcontext_msg(const char *fmt,...)
 {
 	ErrorData  *edata = &errordata[errordata_stack_depth];
@@ -1105,7 +1089,6 @@ errcontext_msg(const char *fmt,...)
 
 	MemoryContextSwitchTo(oldcontext);
 	recursion_depth--;
-	return 0;					/* return value does not matter */
 }
 
 /*
@@ -1117,7 +1100,7 @@ errcontext_msg(const char *fmt,...)
  * a set_errcontext_domain() call to specify the domain.  This is usually
  * done transparently by the errcontext() macro.
  */
-int
+void
 set_errcontext_domain(const char *domain)
 {
 	ErrorData  *edata = &errordata[errordata_stack_depth];
@@ -1127,8 +1110,6 @@ set_errcontext_domain(const char *domain)
 
 	/* the default text domain is the backend's */
 	edata->context_domain = domain ? domain : PG_TEXTDOMAIN("postgres");
-
-	return 0;					/* return value does not matter */
 }
 
 
@@ -1137,7 +1118,7 @@ set_errcontext_domain(const char *domain)
  *
  * This should be called if the message text already includes the statement.
  */
-int
+void
 errhidestmt(bool hide_stmt)
 {
 	ErrorData  *edata = &errordata[errordata_stack_depth];
@@ -1146,8 +1127,6 @@ errhidestmt(bool hide_stmt)
 	CHECK_STACK_DEPTH();
 
 	edata->hide_stmt = hide_stmt;
-
-	return 0;					/* return value does not matter */
 }
 
 /*
@@ -1156,7 +1135,7 @@ errhidestmt(bool hide_stmt)
  * This should only be used for verbose debugging messages where the repeated
  * inclusion of context would bloat the log volume too much.
  */
-int
+void
 errhidecontext(bool hide_ctx)
 {
 	ErrorData  *edata = &errordata[errordata_stack_depth];
@@ -1165,8 +1144,6 @@ errhidecontext(bool hide_ctx)
 	CHECK_STACK_DEPTH();
 
 	edata->hide_ctx = hide_ctx;
-
-	return 0;					/* return value does not matter */
 }
 
 
@@ -1177,7 +1154,7 @@ errhidecontext(bool hide_ctx)
  * name appear in messages sent to old-protocol clients.  Note that the
  * passed string is expected to be a non-freeable constant string.
  */
-int
+void
 errfunction(const char *funcname)
 {
 	ErrorData  *edata = &errordata[errordata_stack_depth];
@@ -1187,14 +1164,12 @@ errfunction(const char *funcname)
 
 	edata->funcname = funcname;
 	edata->show_funcname = true;
-
-	return 0;					/* return value does not matter */
 }
 
 /*
  * errposition --- add cursor position to the current error
  */
-int
+void
 errposition(int cursorpos)
 {
 	ErrorData  *edata = &errordata[errordata_stack_depth];
@@ -1203,14 +1178,12 @@ errposition(int cursorpos)
 	CHECK_STACK_DEPTH();
 
 	edata->cursorpos = cursorpos;
-
-	return 0;					/* return value does not matter */
 }
 
 /*
  * internalerrposition --- add internal cursor position to the current error
  */
-int
+void
 internalerrposition(int cursorpos)
 {
 	ErrorData  *edata = &errordata[errordata_stack_depth];
@@ -1219,8 +1192,6 @@ internalerrposition(int cursorpos)
 	CHECK_STACK_DEPTH();
 
 	edata->internalpos = cursorpos;
-
-	return 0;					/* return value does not matter */
 }
 
 /*
@@ -1230,7 +1201,7 @@ internalerrposition(int cursorpos)
  * is intended for use in error callback subroutines that are editorializing
  * on the layout of the error report.
  */
-int
+void
 internalerrquery(const char *query)
 {
 	ErrorData  *edata = &errordata[errordata_stack_depth];
@@ -1246,8 +1217,6 @@ internalerrquery(const char *query)
 
 	if (query)
 		edata->internalquery = MemoryContextStrdup(edata->assoc_context, query);
-
-	return 0;					/* return value does not matter */
 }
 
 /*
@@ -1260,7 +1229,7 @@ internalerrquery(const char *query)
  * Most potential callers should not use this directly, but instead prefer
  * higher-level abstractions, such as errtablecol() (see relcache.c).
  */
-int
+void
 err_generic_string(int field, const char *str)
 {
 	ErrorData  *edata = &errordata[errordata_stack_depth];
@@ -1289,8 +1258,6 @@ err_generic_string(int field, const char *str)
 			elog(ERROR, "unsupported ErrorData field id: %d", field);
 			break;
 	}
-
-	return 0;					/* return value does not matter */
 }
 
 /*
@@ -1356,108 +1323,6 @@ getinternalerrposition(void)
 
 
 /*
- * elog_start --- startup for old-style API
- *
- * All that we do here is stash the hidden filename/lineno/funcname
- * arguments into a stack entry, along with the current value of errno.
- *
- * We need this to be separate from elog_finish because there's no other
- * C89-compliant way to deal with inserting extra arguments into the elog
- * call.  (When using C99's __VA_ARGS__, we could possibly merge this with
- * elog_finish, but there doesn't seem to be a good way to save errno before
- * evaluating the format arguments if we do that.)
- */
-void
-elog_start(const char *filename, int lineno, const char *funcname)
-{
-	ErrorData  *edata;
-
-	/* Make sure that memory context initialization has finished */
-	if (ErrorContext == NULL)
-	{
-		/* Oops, hard crash time; very little we can do safely here */
-		write_stderr("error occurred at %s:%d before error message processing is available\n",
-					 filename ? filename : "(unknown file)", lineno);
-		exit(2);
-	}
-
-	if (++errordata_stack_depth >= ERRORDATA_STACK_SIZE)
-	{
-		/*
-		 * Wups, stack not big enough.  We treat this as a PANIC condition
-		 * because it suggests an infinite loop of errors during error
-		 * recovery.  Note that the message is intentionally not localized,
-		 * else failure to convert it to client encoding could cause further
-		 * recursion.
-		 */
-		errordata_stack_depth = -1; /* make room on stack */
-		ereport(PANIC, (errmsg_internal("ERRORDATA_STACK_SIZE exceeded")));
-	}
-
-	edata = &errordata[errordata_stack_depth];
-	if (filename)
-	{
-		const char *slash;
-
-		/* keep only base name, useful especially for vpath builds */
-		slash = strrchr(filename, '/');
-		if (slash)
-			filename = slash + 1;
-	}
-	edata->filename = filename;
-	edata->lineno = lineno;
-	edata->funcname = funcname;
-	/* errno is saved now so that error parameter eval can't change it */
-	edata->saved_errno = errno;
-
-	/* Use ErrorContext for any allocations done at this level. */
-	edata->assoc_context = ErrorContext;
-}
-
-/*
- * elog_finish --- finish up for old-style API
- */
-void
-elog_finish(int elevel, const char *fmt,...)
-{
-	ErrorData  *edata = &errordata[errordata_stack_depth];
-	MemoryContext oldcontext;
-
-	CHECK_STACK_DEPTH();
-
-	/*
-	 * Do errstart() to see if we actually want to report the message.
-	 */
-	errordata_stack_depth--;
-	errno = edata->saved_errno;
-	if (!errstart(elevel, edata->filename, edata->lineno, edata->funcname, NULL))
-		return;					/* nothing to do */
-
-	/*
-	 * Format error message just like errmsg_internal().
-	 */
-	recursion_depth++;
-	oldcontext = MemoryContextSwitchTo(edata->assoc_context);
-
-	if (!edata->backtrace &&
-		edata->funcname &&
-		matches_backtrace_functions(edata->funcname))
-		set_backtrace(edata, 2);
-
-	edata->message_id = fmt;
-	EVALUATE_MESSAGE(edata->domain, message, false, false);
-
-	MemoryContextSwitchTo(oldcontext);
-	recursion_depth--;
-
-	/*
-	 * And let errfinish() finish up.
-	 */
-	errfinish(0);
-}
-
-
-/*
  * Functions to allow construction of error message strings separately from
  * the ereport() call itself.
  *
@@ -1696,8 +1561,7 @@ ThrowErrorData(ErrorData *edata)
 	ErrorData  *newedata;
 	MemoryContext oldcontext;
 
-	if (!errstart(edata->elevel, edata->filename, edata->lineno,
-				  edata->funcname, NULL))
+	if (!errstart(edata->elevel, edata->domain))
 		return;					/* error is not to be reported at all */
 
 	newedata = &errordata[errordata_stack_depth];
@@ -1739,7 +1603,7 @@ ThrowErrorData(ErrorData *edata)
 	recursion_depth--;
 
 	/* Process the error. */
-	errfinish(0);
+	errfinish(edata->filename, edata->lineno, edata->funcname);
 }
 
 /*
@@ -1853,7 +1717,7 @@ pg_re_throw(void)
 		 */
 		error_context_stack = NULL;
 
-		errfinish(0);
+		errfinish(edata->filename, edata->lineno, edata->funcname);
 	}
 
 	/* Doesn't return ... */