On Sat, Dec 17, 2011 at 11:04:43PM -0500, Tom Lane wrote: > Noah Misch <n...@leadboat.com> writes: > > Setting aside whether we should offer a better diagnostic when a user points > > "include" at a directory, the yy_fatal_error hack that made sense for scan.l > > doesn't cut it for guc-file.l. Like other guc-file.l errors, we need to > > choose the elevel based on which process is running the code. ERROR is > > never > > the right choice. We should instead stash the message, longjmp out of the > > flex parser, and proceed to handle the error mostly like a regular syntax > > error. See attached small patch. > > Well, if you're going to criticize the original method as being hackish, > you really need to offer a cleaner substitute than this one ;-). In > particular I'm not happy with adding a sigsetjmp() cycle for every > single token parsed.
On the contrary, I want to make it even more of a hack to get better behavior! Here's a version that calls sigsetjmp() once per file. While postgresql.conf scanning hardly seems like the place to be shaving cycles, this does catch fatal errors in functions other than yylex(), notably yy_create_buffer(). > > On a related note, the out-of-memory handling during config file reload is > > inconsistent. guc-file.l uses palloc/pstrdup in various places. An OOM at > > those sites during a reload would kill the postmaster. > > If you've got OOM in the postmaster, you're dead anyway. I feel no > compulsion to worry about this. Works for me. Thanks, nm
*** a/src/backend/utils/misc/guc-file.l --- b/src/backend/utils/misc/guc-file.l *************** *** 20,28 **** #include "utils/guc.h" ! /* Avoid exit() on fatal scanner errors (a bit ugly -- see yy_fatal_error) */ #undef fprintf ! #define fprintf(file, fmt, msg) ereport(ERROR, (errmsg_internal("%s", msg))) enum { GUC_ID = 1, --- 20,40 ---- #include "utils/guc.h" ! /* ! * flex emits a yy_fatal_error() function that it calls in response to ! * critical errors like malloc failure, file I/O errors, and detection of ! * internal inconsistency. That function prints a message and calls exit(). ! * Mutate it to instead stash the message and jump out of the parser. Assume ! * all msg arguments point to string constants; this holds for flex 2.5.31 ! * (earliest we support) and flex 2.5.35 (latest as of this writing). ! */ ! static const char *GUC_flex_fatal_errmsg; ! static sigjmp_buf *GUC_flex_fatal_jmp; #undef fprintf ! #define fprintf(file, fmt, msg) \ ! 0; /* eat cast to void */ \ ! GUC_flex_fatal_errmsg = msg; \ ! siglongjmp(*GUC_flex_fatal_jmp, 1) enum { GUC_ID = 1, *************** *** 464,482 **** ParseConfigFp(FILE *fp, const char *config_file, int depth, int elevel, ConfigVariable **head_p, ConfigVariable **tail_p) { bool OK = true; ! YY_BUFFER_STATE lex_buffer; int errorcount; int token; /* * Parse */ - lex_buffer = yy_create_buffer(fp, YY_BUF_SIZE); - yy_switch_to_buffer(lex_buffer); - ConfigFileLineno = 1; errorcount = 0; /* This loop iterates once per logical line */ while ((token = yylex())) { --- 476,511 ---- ConfigVariable **head_p, ConfigVariable **tail_p) { bool OK = true; ! volatile YY_BUFFER_STATE lex_buffer = NULL; int errorcount; int token; + sigjmp_buf flex_fatal_jmp; + + if (sigsetjmp(flex_fatal_jmp, 1) == 0) + GUC_flex_fatal_jmp = &flex_fatal_jmp; + else + { + /* + * Regain control after a fatal, internal flex error. It may have + * corrupted parser state. Consequently, abandon the file, but trust + * that the state remains sane enough for yy_delete_buffer(). + */ + elog(elevel, "%s at file \"%s\" line %u", + GUC_flex_fatal_errmsg, config_file, ConfigFileLineno); + + yy_delete_buffer(lex_buffer); + return false; + } /* * Parse */ ConfigFileLineno = 1; errorcount = 0; + lex_buffer = yy_create_buffer(fp, YY_BUF_SIZE); + yy_switch_to_buffer(lex_buffer); + /* This loop iterates once per logical line */ while ((token = yylex())) { *************** *** 532,537 **** ParseConfigFp(FILE *fp, const char *config_file, int depth, int elevel, --- 561,567 ---- depth + 1, elevel, head_p, tail_p)) OK = false; + GUC_flex_fatal_jmp = &flex_fatal_jmp; yy_switch_to_buffer(lex_buffer); ConfigFileLineno = save_ConfigFileLineno; pfree(opt_name); *************** *** 549,554 **** ParseConfigFp(FILE *fp, const char *config_file, int depth, int elevel, --- 579,585 ---- depth + 1, elevel, head_p, tail_p)) OK = false; + GUC_flex_fatal_jmp = &flex_fatal_jmp; yy_switch_to_buffer(lex_buffer); ConfigFileLineno = save_ConfigFileLineno; pfree(opt_name);
-- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs