8000 Minor code refactoring in elog.c (no functional change). · postgres/postgres@8305629 · GitHub
[go: up one dir, main page]
Skip to content

Commit 8305629

Browse files
tglsfdctglsfdc
committed
Minor code refactoring in elog.c (no functional change).
Combine some duplicated code stanzas by creating small functions. Most of these duplications arose at a time when I wouldn't have trusted C compilers to auto-inline small functions intelligently, but they're probably poor practice now. Similarly split out some bits that aren't actually duplicative as the code stands, but would become so after an upcoming patch to add another error-handling code path. Take the opportunity to add some lengthier comments about what we're doing here, too. Re-order one function that seemed not very well-placed. Patch by me, per suggestions from Andres Freund. Discussion: https://postgr.es/m/3bbbb0df-7382-bf87-9737-340ba096e034@postgrespro.ru
1 parent 3b31821 commit 8305629

File tree

1 file changed

+141
-116
lines changed

1 file changed

+141
-116
lines changed

‎src/backend/utils/error/elog.c‎

Lines changed: 141 additions & 116 deletions
B5FA
Original file line numberDiff line numberDiff line change
@@ -176,8 +176,15 @@ static char formatted_log_time[FORMATTED_TS_LEN];
176176

177177

178178
static const char *err_gettext(const char *str) pg_attribute_format_arg(1);
179+
static ErrorData *get_error_stack_entry(void);
180+
static void set_stack_entry_domain(ErrorData *edata, const char *domain);
181+
static void set_stack_entry_location(ErrorData *edata,
182+
const char *filename, int lineno,
183+
const char *funcname);
184+
static bool matches_backtrace_functions(const char *funcname);
179185
static pg_noinline void set_backtrace(ErrorData *edata, int num_skip);
180186
static void set_errdata_field(MemoryContextData *cxt, char **ptr, const char *str);
187+
static void FreeErrorDataContents(ErrorData *edata);
181188
static void write_console(const char *line, int len);
182189
static const char *process_log_prefix_padding(const char *p, int *ppadding);
183190
static void log_line_prefix(StringInfo buf, ErrorData *edata);
@@ -434,36 +441,20 @@ errstart(int elevel, const char *domain)
434441
debug_query_string = NULL;
435442
}
436443
}
437-
if (++errordata_stack_depth >= ERRORDATA_STACK_SIZE)
438-
{
439-
/*
440-
* Wups, stack not big enough. We treat this as a PANIC condition
441-
* because it suggests an infinite loop of errors during error
442-
* recovery.
443-
*/
444-
errordata_stack_depth = -1; /* make room on stack */
445-
ereport(PANIC, (errmsg_internal("ERRORDATA_STACK_SIZE exceeded")));
446-
}
447444

448445
/* Initialize data for this error frame */
449-
edata = &errordata[errordata_stack_depth];
450-
MemSet(edata, 0, sizeof(ErrorData));
446+
edata = get_error_stack_entry();
451447
edata->elevel = elevel;
452448
edata->output_to_server = output_to_server;
453449
edata->output_to_client = output_to_client;
454-
/* the default text domain is the backend's */
455-
edata->domain = domain ? domain : PG_TEXTDOMAIN("postgres");
456-
/* initialize context_domain the same way (see set_errcontext_domain()) */
457-
edata->context_domain = edata->domain;
450+
set_stack_entry_domain(edata, domain);
458451
/* Select default errcode based on elevel */
459452
if (elevel >= ERROR)
460453
edata->sqlerrcode = ERRCODE_INTERNAL_ERROR;
461454
else if (elevel >= WARNING)
462455
edata->sqlerrcode = ERRCODE_WARNING;
463456
else
464457
edata->sqlerrcode = ERRCODE_SUCCESSFUL_COMPLETION;
465-
/* errno is saved here so that error parameter eval can't change it */
466-
edata->saved_errno = errno;
467458

468459
/*
469460
* Any allocations for this error state level should go into ErrorContext
@@ -474,32 +465,6 @@ errstart(int elevel, const char *domain)
474465
return true;
475466
}
476467

477-
/*
478-
* Checks whether the given funcname matches backtrace_functions; see
479-
* check_backtrace_functions.
480-
*/
481-
static bool
482-
matches_backtrace_functions(const char *funcname)
483-
{
484-
char *p;
485-
486-
if (!backtrace_symbol_list || funcname == NULL || funcname[0] == '\0')
487-
return false;
488-
489-
p = backtrace_symbol_list;
490-
for (;;)
491-
{
492-
if (*p == '\0') /* end of backtrace_symbol_list */
493-
break;
494-
495-
if (strcmp(funcname, p) == 0)
496-
return true;
497-
p += strlen(p) + 1;
498-
}
499-
500-
return false;
501-
}
502-
503468
/*
504469
* errfinish --- end an error-reporting cycle
505470
*
@@ -520,23 +485,7 @@ errfinish(const char *filename, int lineno, const char *funcname)
520485
CHECK_STACK_DEPTH();
521486

522487
/* Save the last few bits of error state into the stack entry */
523-
if (filename)
524-
{
525-
const char *slash;
526-
527-
/* keep only base name, useful especially for vpath builds */
528-
slash = strrchr(filename, '/');
529-
if (slash)
530-
filename = slash + 1;
531-
/* Some Windows compilers use backslashes in __FILE__ strings */
532-
slash = strrchr(filename, '\\');
533-
if (slash)
534-
filename = slash + 1;
535-
}
536-
537-
edata->filename = filename;
538-
edata->lineno = lineno;
539-
edata->funcname = funcname;
488+
set_stack_entry_location(edata, filename, lineno, funcname);
540489

541490
elevel = edata->elevel;
542491

@@ -546,6 +495,7 @@ errfinish(const char *filename, int lineno, const char *funcname)
546495
*/
547496
oldcontext = MemoryContextSwitchTo(ErrorContext);
548497

498+
/* Collect backtrace, if enabled and we didn't already */
549499
if (!edata->backtrace &&
550500
edata->funcname &&
551501
backtrace_functions &&
@@ -596,31 +546,7 @@ errfinish(const char *filename, int lineno, const char *funcname)
596546
EmitErrorReport();
597547

598548
/* Now free up subsidiary data attached to stack entry, and release it */
599-
if (edata->message)
600-
pfree(edata->message);
601-
if (edata->detail)
602-
pfree(edata->detail);
603-
if (edata->detail_log)
604-
pfree(edata->detail_log);
605-
if (edata->hint)
606-
pfree(edata->hint);
607-
if (edata->context)
608-
pfree(edata->context);
609-
if (edata->backtrace)
610-
pfree(edata->backtrace);
611-
if (edata->schema_name)
612-
pfree(edata->schema_name);
613-
if (edata->table_name)
614-
pfree(edata->table_name);
615-
if (edata->column_name)
616-
pfree(edata->column_name);
617-
if (edata->datatype_name)
618-
pfree(edata->datatype_name);
619-
if (edata->constraint_name)
620-
pfree(edata->constraint_name);
621-
if (edata->internalquery)
622-
pfree(edata->internalquery);
623-
549+
FreeErrorDataContents(edata);
624550
errordata_stack_depth--;
625551

626552
/* Exit error-handling context */
@@ -685,6 +611,120 @@ errfinish(const char *filename, int lineno, const char *funcname)
685611
CHECK_FOR_INTERRUPTS();
686612
}
687613

614+
/*
615+
* get_error_stack_entry --- allocate and initialize a new stack entry
616+
*
617+
* The entry should be freed, when we're done with it, by calling
618+
* FreeErrorDataContents() and then decrementing errordata_stack_depth.
619+
*
620+
* Returning the entry's address is just a notational convenience,
621+
* since it had better be errordata[errordata_stack_depth].
622+
*
623+
* Although the error stack is not large, we don't expect to run out of space.
624+
* Using more than one entry implies a new error report during error recovery,
625+
* which is possible but already suggests we're in trouble. If we exhaust the
626+
* stack, almost certainly we are in an infinite loop of errors during error
627+
* recovery, so we give up and PANIC.
628+
*
629+
* (Note that this is distinct from the recursion_depth checks, which
630+
* guard against recursion while handling a single stack entry.)
631+
*/
632+
static ErrorData *
633+
get_error_stack_entry(void)
634+
{
635+
ErrorData *edata;
636+
637+
/* Allocate error frame */
638+
errordata_stack_depth++;
639+
if (unlikely(errordata_stack_depth >= ERRORDATA_STACK_SIZE))
640+
{
641+
/* Wups, stack not big enough */
642+
errordata_stack_depth = -1; /* make room on stack */
643+
ereport(PANIC, (errmsg_internal("ERRORDATA_STACK_SIZE exceeded")));
644+
}
645+
646+
/* Initialize error frame to all zeroes/NULLs */
647+
edata = &errordata[errordata_stack_depth];
648+
memset(edata, 0, sizeof(ErrorData));
649+
650+
/* Save errno immediately to ensure error parameter eval can't change it */
651+
edata->saved_errno = errno;
652+
653+
return edata;
654+
}
655+
656+
/*
657+
* set_stack_entry_domain --- fill in the internationalization domain
658+
*/
659+
static void
660+
set_stack_entry_domain(ErrorData *edata, const char *domain)
661+
{
662+
/* the default text domain is the backend's */
663+
edata->domain = domain ? domain : PG_TEXTDOMAIN("postgres");
664+
/* initialize context_domain the same way (see set_errcontext_domain()) */
665+
edata->context_domain = edata->domain;
666+
}
667+
668+
/*
669+
* set_stack_entry_location --- fill in code-location details
670+
*
671+
* Store the values of __FILE__, __LINE__, and __func__ from the call site.
672+
* We make an effort to normalize __FILE__, since compilers are inconsistent
673+
* about how much of the path they'll include, and we'd prefer that the
674+
* behavior not depend on that (especially, that it not vary with build path).
675+
*/
676+
static void
677+
set_stack_entry_location(ErrorData *edata,
678+
const char *filename, int lineno,
679+
const char *funcname)
680+
{
681+
if (filename)
682+
{
683+
const char *slash;
684+
685+
/* keep only base name, useful especially for vpath builds */
686+
slash = strrchr(filename, '/');
687+
if (slash)
688+
filename = slash + 1;
689+
/* Some Windows compilers use backslashes in __FILE__ strings */
690+
slash = strrchr(filename, '\\');
691+
if (slash)
692+
filename = slash + 1;
693+
}
694+
695+
edata->filename = filename;
696+
edata->lineno = lineno;
697+
edata->funcname = funcname;
698+
}
699+
700+
/*
701+
* matches_backtrace_functions --- checks whether the given funcname matches
702+
* backtrace_functions
703+
*
704+
* See check_backtrace_functions.
705+
*/
706+
static bool
707+
matches_backtrace_functions(const char *funcname)
708+
{
709+
const char *p;
710+
711+
if (!backtrace_symbol_list || funcname == NULL || funcname[0] == '\0')
712+
return false;
713+
714+
p = backtrace_symbol_list;
715+
for (;;)
716+
{
717+
if (*p == '\0') /* end of backtrace_symbol_list */
718+
break;
719+
720+
if (strcmp(funcname, p) == 0)
721+
return true;
722+
p += strlen(p) + 1;
723+
}
724+
725+
return false;
726+
}
727+
688728

689729
/*
690730
* errcode --- add SQLSTATE error code to the current error
@@ -1611,6 +1651,18 @@ CopyErrorData(void)
16111651
*/
16121652
void
16131653
FreeErrorData(ErrorData *edata)
1654+
{
1655+
FreeErrorDataContents(edata);
1656+
pfree(edata);
1657+
}
1658+
1659+
/*
1660+
* FreeErrorDataContents --- free the subsidiary data of an ErrorData.
1661+
*
1662+
* This can be used on either an error stack entry or a copied ErrorData.
1663+
*/
1664+
static void
1665+
FreeErrorDataContents(ErrorData *edata)
16141666
{
16151667
if (edata->message)
16161668
pfree(edata->message);
@@ -1636,7 +1688,6 @@ FreeErrorData(ErrorData *edata)
16361688
pfree(edata->constraint_name);
16371689
if (edata->internalquery)
16381690
pfree(edata->internalquery);
1639-
pfree(edata);
16401691
}
16411692

16421693
/*
@@ -1742,18 +1793,7 @@ ReThrowError(ErrorData *edata)
17421793
recursion_depth++;
17431794
MemoryContextSwitchTo(ErrorContext);
17441795

1745-
if (++errordata_stack_depth >= ERRORDATA_STACK_SIZE)
1746-
{
1747-
/*
1748-
* Wups, stack not big enough. We treat this as a PANIC condition
1749-
* because it suggests an infinite loop of errors during error
1750-
* recovery.
1751-
*/
1752-
errordata_stack_depth = -1; /* make room on stack */
1753-
ereport(PANIC, (errmsg_internal("ERRORDATA_STACK_SIZE exceeded")));
1754-
}
1755-
1756-
newedata = &errordata[errordata_stack_depth];
1796+
newedata = get_error_stack_entry();
17571797
memcpy(newedata, edata, sizeof(ErrorData));
17581798

17591799
/* Make copies of separately-allocated fields */
@@ -1854,26 +1894,11 @@ GetErrorContextStack(void)
18541894
ErrorContextCallback *econtext;
18551895

18561896
/*
1857-
* Okay, crank up a stack entry to store the info in.
1897+
* Crank up a stack entry to store the info in.
18581898
*/
18591899
recursion_depth++;
18601900

1861-
if (++errordata_stack_depth >= ERRORDATA_STACK_SIZE)
1862-
{
1863-
/*
1864-
* Wups, stack not big enough. We treat this as a PANIC condition
1865-
* because it suggests an infinite loop of errors during error
1866-
* recovery.
1867-
*/
1868-
errordata_stack_depth = -1; /* make room on stack */
1869-
ereport(PANIC, (errmsg_internal("ERRORDATA_STACK_SIZE exceeded")));
1870-
}
1871-
1872-
/*
1873-
* Things look good so far, so initialize our error frame
1874-
*/
1875-
edata = &errordata[errordata_stack_depth];
1876-
MemSet(edata, 0, sizeof(ErrorData));
1901+
edata = get_error_stack_entry();
18771902

18781903
/*
18791904
* Set up assoc_context to be the caller's context, so any allocations

0 commit comments

Comments
 (0)
0