From a09dcf6db3b0aff56e2861e2d952fb84ffd35d80 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 25 Mar 2020 11:57:36 -0400 Subject: [PATCH] Go back to returning int from ereport auxiliary functions. This reverts the parts of commit 17a28b03645e27d73bf69a95d7569b61e58f06eb that changed ereport's auxiliary functions from returning dummy integer values to returning void. It turns out that a minority of compilers complain (not entirely unreasonably) about constructs such as (condition) ? errdetail(...) : 0 if errdetail() returns void rather than int. We could update those call sites to say "(void) 0" perhaps, but the expectation for this patch set was that ereport callers would not have to change anything. And this aspect of the patch set was already the most invasive and least compelling part of it, so let's just drop it. Per buildfarm. Discussion: https://postgr.es/m/CA+fd4k6N8EjNvZpM8nme+y+05mz-SM8Z_BgkixzkA34R+ej0Kw@mail.gmail.com --- src/backend/parser/parse_coerce.c | 6 +-- src/backend/parser/parse_node.c | 8 ++-- src/backend/storage/ipc/dsm_impl.c | 8 ++-- src/backend/utils/error/elog.c | 60 +++++++++++++++++++++--------- src/include/executor/executor.h | 2 +- src/include/parser/parse_coerce.h | 2 +- src/include/parser/parse_node.h | 2 +- src/include/parser/scanner.h | 1 - src/include/utils/elog.h | 38 +++++++++---------- src/pl/plpgsql/src/pl_scanner.c | 6 +-- src/pl/plpgsql/src/plpgsql.h | 2 +- 11 files changed, 79 insertions(+), 56 deletions(-) diff --git a/src/backend/parser/parse_coerce.c b/src/backend/parser/parse_coerce.c index 3a5db14cd64..bca5976e649 100644 --- a/src/backend/parser/parse_coerce.c +++ b/src/backend/parser/parse_coerce.c @@ -1317,15 +1317,15 @@ coerce_to_specific_type(ParseState *pstate, Node *node, * XXX possibly this is more generally useful than coercion errors; * if so, should rename and place with parser_errposition. */ -void +int parser_coercion_errposition(ParseState *pstate, int coerce_location, Node *input_expr) { if (coerce_location >= 0) - parser_errposition(pstate, coerce_location); + return parser_errposition(pstate, coerce_location); else - parser_errposition(pstate, exprLocation(input_expr)); + return parser_errposition(pstate, exprLocation(input_expr)); } diff --git a/src/backend/parser/parse_node.c b/src/backend/parser/parse_node.c index 6a7d31a18da..e6191e35659 100644 --- a/src/backend/parser/parse_node.c +++ b/src/backend/parser/parse_node.c @@ -107,21 +107,21 @@ free_parsestate(ParseState *pstate) * normal non-error case: computing character indexes would be much more * expensive than storing token offsets.) */ -void +int parser_errposition(ParseState *pstate, int location) { int pos; /* No-op if location was not provided */ if (location < 0) - return; + return 0; /* Can't do anything if source text is not available */ if (pstate == NULL || pstate->p_sourcetext == NULL) - return; + return 0; /* Convert offset to character number */ pos = pg_mbstrlen_with_len(pstate->p_sourcetext, location) + 1; /* And pass it to the ereport mechanism */ - errposition(pos); + return errposition(pos); } diff --git a/src/backend/storage/ipc/dsm_impl.c b/src/backend/storage/ipc/dsm_impl.c index 46a60e1b898..5dac6e0653d 100644 --- a/src/backend/storage/ipc/dsm_impl.c +++ b/src/backend/storage/ipc/dsm_impl.c @@ -94,7 +94,7 @@ static bool dsm_impl_mmap(dsm_op op, dsm_handle handle, Size request_size, void **impl_private, void **mapped_address, Size *mapped_size, int elevel); #endif -static void errcode_for_dynamic_shared_memory(void); +static int errcode_for_dynamic_shared_memory(void); const struct config_enum_entry dynamic_shared_memory_options[] = { #ifdef USE_DSM_POSIX @@ -1048,11 +1048,11 @@ dsm_impl_unpin_segment(dsm_handle handle, void **impl_private) } } -static void +static int errcode_for_dynamic_shared_memory(void) { if (errno == EFBIG || errno == ENOMEM) - errcode(ERRCODE_OUT_OF_MEMORY); + return errcode(ERRCODE_OUT_OF_MEMORY); else - errcode_for_file_access(); + return errcode_for_file_access(); } diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c index 859ac9e56fa..21bd3c8b164 100644 --- a/src/backend/utils/error/elog.c +++ b/src/backend/utils/error/elog.c @@ -898,7 +898,7 @@ errfinish_and_return(const char *filename, int lineno, const char *funcname) * * The code is expected to be represented as per MAKE_SQLSTATE(). */ -void +int errcode(int sqlerrcode) { ErrorData *edata = &errordata[errordata_stack_depth]; @@ -913,6 +913,7 @@ errcode(int sqlerrcode) edata->omit_location = false; else edata->omit_location = true; + return 0; /* return value does not matter */ } @@ -925,7 +926,7 @@ errcode(int sqlerrcode) * NOTE: the primary error message string should generally include %m * when this is used. */ -void +int errcode_for_file_access(void) { ErrorData *edata = &errordata[errordata_stack_depth]; @@ -984,6 +985,8 @@ errcode_for_file_access(void) edata->omit_location = false; break; } + + return 0; /* return value does not matter */ } /* @@ -995,7 +998,7 @@ errcode_for_file_access(void) * NOTE: the primary error message string should generally include %m * when this is used. */ -void +int errcode_for_socket_access(void) { ErrorData *edata = &errordata[errordata_stack_depth]; @@ -1016,6 +1019,8 @@ errcode_for_socket_access(void) edata->omit_location = false; break; } + + return 0; /* return value does not matter */ } /* @@ -1137,7 +1142,7 @@ sqlstate_to_errcode(const char *sqlstate) * Note: no newline is needed at the end of the fmt string, since * ereport will provide one for the output methods that need it. */ -void +int errmsg(const char *fmt,...) { ErrorData *edata = &errordata[errordata_stack_depth]; @@ -1153,6 +1158,7 @@ errmsg(const char *fmt,...) MemoryContextSwitchTo(oldcontext); recursion_depth--; errno = edata->saved_errno; /*CDB*/ + return 0; /* return value does not matter */ } /* @@ -1224,7 +1230,7 @@ set_backtrace(ErrorData *edata, int num_skip) * the message because the translation would fail and result in infinite * error recursion. */ -void +int errmsg_internal(const char *fmt,...) { ErrorData *edata = &errordata[errordata_stack_depth]; @@ -1240,6 +1246,7 @@ errmsg_internal(const char *fmt,...) MemoryContextSwitchTo(oldcontext); recursion_depth--; errno = edata->saved_errno; /*CDB*/ + return 0; /* return value does not matter */ } @@ -1247,7 +1254,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 */ -void +int errmsg_plural(const char *fmt_singular, const char *fmt_plural, unsigned long n, ...) { @@ -1264,13 +1271,14 @@ errmsg_plural(const char *fmt_singular, const char *fmt_plural, MemoryContextSwitchTo(oldcontext); recursion_depth--; errno = edata->saved_errno; /*CDB*/ + return 0; /* return value does not matter */ } /* * errdetail --- add a detail error message text to the current error */ -void +int errdetail(const char *fmt,...) { ErrorData *edata = &errordata[errordata_stack_depth]; @@ -1285,6 +1293,7 @@ errdetail(const char *fmt,...) MemoryContextSwitchTo(oldcontext); recursion_depth--; errno = edata->saved_errno; /*CDB*/ + return 0; /* return value does not matter */ } @@ -1297,7 +1306,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). */ -void +int errdetail_internal(const char *fmt,...) { ErrorData *edata = &errordata[errordata_stack_depth]; @@ -1311,13 +1320,14 @@ 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 */ -void +int errdetail_log(const char *fmt,...) { ErrorData *edata = &errordata[errordata_stack_depth]; @@ -1332,13 +1342,14 @@ errdetail_log(const char *fmt,...) MemoryContextSwitchTo(oldcontext); recursion_depth--; errno = edata->saved_errno; /*CDB*/ + 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 */ -void +int errdetail_log_plural(const char *fmt_singular, const char *fmt_plural, unsigned long n,...) { @@ -1353,6 +1364,7 @@ errdetail_log_plural(const char *fmt_singular, const char *fmt_plural, MemoryContextSwitchTo(oldcontext); recursion_depth--; + return 0; /* return value does not matter */ } @@ -1360,7 +1372,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 */ -void +int errdetail_plural(const char *fmt_singular, const char *fmt_plural, unsigned long n, ...) { @@ -1376,13 +1388,14 @@ errdetail_plural(const char *fmt_singular, const char *fmt_plural, MemoryContextSwitchTo(oldcontext); recursion_depth--; errno = edata->saved_errno; /*CDB*/ + return 0; /* return value does not matter */ } /* * errhint --- add a hint error message text to the current error */ -void +int errhint(const char *fmt,...) { ErrorData *edata = &errordata[errordata_stack_depth]; @@ -1397,6 +1410,7 @@ errhint(const char *fmt,...) MemoryContextSwitchTo(oldcontext); recursion_depth--; errno = edata->saved_errno; /*CDB*/ + return 0; /* return value does not matter */ } @@ -1477,7 +1491,7 @@ set_errcontext_domain(const char *domain) * * This should be called if the message text already includes the statement. */ -void +int errhidestmt(bool hide_stmt) { ErrorData *edata = &errordata[errordata_stack_depth]; @@ -1486,6 +1500,8 @@ errhidestmt(bool hide_stmt) CHECK_STACK_DEPTH(); edata->hide_stmt = hide_stmt; + + return 0; /* return value does not matter */ } /* @@ -1494,7 +1510,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. */ -void +int errhidecontext(bool hide_ctx) { ErrorData *edata = &errordata[errordata_stack_depth]; @@ -1503,6 +1519,8 @@ errhidecontext(bool hide_ctx) CHECK_STACK_DEPTH(); edata->hide_ctx = hide_ctx; + + return 0; /* return value does not matter */ } @@ -1519,7 +1537,7 @@ errposition(int cursorpos) edata->cursorpos = cursorpos; - return 0; + return 0; /* return value does not matter */ } /* @@ -1538,7 +1556,7 @@ errprintstack(bool printstack) /* * internalerrposition --- add internal cursor position to the current error */ -void +int internalerrposition(int cursorpos) { ErrorData *edata = &errordata[errordata_stack_depth]; @@ -1547,6 +1565,8 @@ internalerrposition(int cursorpos) CHECK_STACK_DEPTH(); edata->internalpos = cursorpos; + + return 0; /* return value does not matter */ } /* @@ -1556,7 +1576,7 @@ internalerrposition(int cursorpos) * is intended for use in error callback subroutines that are editorializing * on the layout of the error report. */ -void +int internalerrquery(const char *query) { ErrorData *edata = &errordata[errordata_stack_depth]; @@ -1573,6 +1593,8 @@ internalerrquery(const char *query) if (query) edata->internalquery = MemoryContextStrdup(edata->assoc_context, query); errno = edata->saved_errno; /*CDB*/ + + return 0; /* return value does not matter */ } /* @@ -1585,7 +1607,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). */ -void +int err_generic_string(int field, const char *str) { ErrorData *edata = &errordata[errordata_stack_depth]; @@ -1614,6 +1636,8 @@ err_generic_string(int field, const char *str) elog(ERROR, "unsupported ErrorData field id: %d", field); break; } + + return 0; /* return value does not matter */ } /* diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index 6b8dbbc9916..5116ba49856 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -623,7 +623,7 @@ extern Relation ExecGetRangeTableRelation(EState *estate, Index rti); extern void ExecInitResultRelation(EState *estate, ResultRelInfo *resultRelInfo, Index rti); -extern int executor_errposition(EState *estate, int location); +extern int executor_errposition(EState *estate, int location); extern void RegisterExprContextCallback(ExprContext *econtext, ExprContextCallbackFunction function, diff --git a/src/include/parser/parse_coerce.h b/src/include/parser/parse_coerce.h index 39c73d5be8c..e954f09fe19 100644 --- a/src/include/parser/parse_coerce.h +++ b/src/include/parser/parse_coerce.h @@ -65,7 +65,7 @@ extern Node *coerce_to_specific_type_typmod(ParseState *pstate, Node *node, Oid targetTypeId, int32 targetTypmod, const char *constructName); -extern void parser_coercion_errposition(ParseState *pstate, +extern int parser_coercion_errposition(ParseState *pstate, int coerce_location, Node *input_expr); diff --git a/src/include/parser/parse_node.h b/src/include/parser/parse_node.h index ef52f016e28..73d6c2cd45c 100644 --- a/src/include/parser/parse_node.h +++ b/src/include/parser/parse_node.h @@ -333,7 +333,7 @@ typedef struct ParseCallbackState extern ParseState *make_parsestate(ParseState *parentParseState); extern void free_parsestate(ParseState *pstate); extern struct HTAB *parser_get_namecache(ParseState *pstate); -extern void parser_errposition(ParseState *pstate, int location); +extern int parser_errposition(ParseState *pstate, int location); extern void setup_parser_errposition_callback(ParseCallbackState *pcbstate, ParseState *pstate, int location); diff --git a/src/include/parser/scanner.h b/src/include/parser/scanner.h index d18689dfd51..0d8182faa0d 100644 --- a/src/include/parser/scanner.h +++ b/src/include/parser/scanner.h @@ -140,7 +140,6 @@ extern core_yyscan_t scanner_init(const char *str, extern void scanner_finish(core_yyscan_t yyscanner); extern int core_yylex(core_YYSTYPE *lvalp, YYLTYPE *llocp, core_yyscan_t yyscanner); - extern int scanner_errposition(int location, core_yyscan_t yyscanner); extern void setup_scanner_errposition_callback(ScannerCallbackState *scbstate, core_yyscan_t yyscanner, diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h index 0f61a3ae8b9..9bfb7c7bbdb 100644 --- a/src/include/utils/elog.h +++ b/src/include/utils/elog.h @@ -203,33 +203,33 @@ extern bool errstart(int elevel, const char *domain); extern pg_attribute_cold bool errstart_cold(int elevel, const char *domain); extern void errfinish(const char *filename, int lineno, const char *funcname); -extern void errcode(int sqlerrcode); +extern int errcode(int sqlerrcode); -extern void errcode_for_file_access(void); -extern void errcode_for_socket_access(void); +extern int errcode_for_file_access(void); +extern int errcode_for_socket_access(void); extern int sqlstate_to_errcode(const char *sqlstate); extern void errcode_to_sqlstate(int errcode, char outbuf[6]); -extern void errmsg(const char *fmt,...) pg_attribute_printf(1, 2); -extern void errmsg_internal(const char *fmt,...) pg_attribute_printf(1, 2); +extern int errmsg(const char *fmt,...) pg_attribute_printf(1, 2); +extern int errmsg_internal(const char *fmt,...) pg_attribute_printf(1, 2); -extern void errmsg_plural(const char *fmt_singular, const char *fmt_plural, +extern int errmsg_plural(const char *fmt_singular, const char *fmt_plural, unsigned long n,...) pg_attribute_printf(1, 4) pg_attribute_printf(2, 4); -extern void errdetail(const char *fmt,...) pg_attribute_printf(1, 2); -extern void errdetail_internal(const char *fmt,...) pg_attribute_printf(1, 2); +extern int errdetail(const char *fmt,...) pg_attribute_printf(1, 2); +extern int errdetail_internal(const char *fmt,...) pg_attribute_printf(1, 2); -extern void errdetail_log(const char *fmt,...) pg_attribute_printf(1, 2); +extern int errdetail_log(const char *fmt,...) pg_attribute_printf(1, 2); -extern void errdetail_log_plural(const char *fmt_singular, +extern int errdetail_log_plural(const char *fmt_singular, const char *fmt_plural, unsigned long n,...) pg_attribute_printf(1, 4) pg_attribute_printf(2, 4); -extern void errdetail_plural(const char *fmt_singular, const char *fmt_plural, +extern int errdetail_plural(const char *fmt_singular, const char *fmt_plural, unsigned long n,...) pg_attribute_printf(1, 4) pg_attribute_printf(2, 4); -extern void errhint(const char *fmt,...) pg_attribute_printf(1, 2); +extern int errhint(const char *fmt,...) pg_attribute_printf(1, 2); extern int errhint_plural(const char *fmt_singular, const char *fmt_plural, unsigned long n,...) pg_attribute_printf(1, 4) pg_attribute_printf(2, 4); @@ -244,12 +244,12 @@ extern int errhint_plural(const char *fmt_singular, const char *fmt_plural, */ #define errcontext set_errcontext_domain(TEXTDOMAIN), errcontext_msg -extern int set_errcontext_domain(const char *domain); +extern int set_errcontext_domain(const char *domain); -extern int errcontext_msg(const char *fmt,...) pg_attribute_printf(1, 2); +extern int errcontext_msg(const char *fmt,...) pg_attribute_printf(1, 2); -extern void errhidestmt(bool hide_stmt); -extern void errhidecontext(bool hide_ctx); +extern int errhidestmt(bool hide_stmt); +extern int errhidecontext(bool hide_ctx); extern int errprintstack(bool printstack); @@ -257,10 +257,10 @@ extern int errbacktrace(void); extern int errposition(int cursorpos); -extern void internalerrposition(int cursorpos); -extern void internalerrquery(const char *query); +extern int internalerrposition(int cursorpos); +extern int internalerrquery(const char *query); -extern void err_generic_string(int field, const char *str); +extern int err_generic_string(int field, const char *str); extern int geterrcode(void); extern int geterrposition(void); diff --git a/src/pl/plpgsql/src/pl_scanner.c b/src/pl/plpgsql/src/pl_scanner.c index 688977e20ef..57f414135e1 100644 --- a/src/pl/plpgsql/src/pl_scanner.c +++ b/src/pl/plpgsql/src/pl_scanner.c @@ -471,20 +471,20 @@ plpgsql_peek2(int *tok1_p, int *tok2_p, int *tok1_loc, int *tok2_loc) * parsing of a plpgsql function, since it requires the scanorig string * to still be available. */ -void +int plpgsql_scanner_errposition(int location) { int pos; if (location < 0 || scanorig == NULL) - return; /* no-op if location is unknown */ + return 0; /* no-op if location is unknown */ /* Convert byte offset to character number */ pos = pg_mbstrlen_with_len(scanorig, location) + 1; /* And pass it to the ereport mechanism */ (void) internalerrposition(pos); /* Also pass the function body string */ - internalerrquery(scanorig); + return internalerrquery(scanorig); } /* diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h index 00756d1b9df..fe99d4d6683 100644 --- a/src/pl/plpgsql/src/plpgsql.h +++ b/src/pl/plpgsql/src/plpgsql.h @@ -1311,7 +1311,7 @@ extern void plpgsql_append_source_text(StringInfo buf, extern int plpgsql_peek(void); extern void plpgsql_peek2(int *tok1_p, int *tok2_p, int *tok1_loc, int *tok2_loc); -extern void plpgsql_scanner_errposition(int location); +extern int plpgsql_scanner_errposition(int location); extern void plpgsql_yyerror(const char *message) pg_attribute_noreturn(); extern int plpgsql_location_to_lineno(int location); extern int plpgsql_latest_lineno(void);