Re: fatal flex error in guc-file.l kills the postmaster

On Sun, Dec 18, 2011 at 11:53 AM, Noah Misch <noah@leadboat.com> wrote:
> Here's a version that calls sigsetjmp() once per file. =A0While postgresq=
l.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().

This strikes me as more clever than necessary:

#define fprintf(file, fmt, msg) \
    0; /* eat cast to void */ \
    GUC_flex_fatal_errmsg =3D msg; \
    siglongjmp(*GUC_flex_fatal_jmp, 1)

Can't we just define a function jumpoutoftheparser() here and call
that function rather than doing that /* eat cast to void */ hack?
That would also involve fewer assumptions about the coding style
emitted by flex.  For example, if flex chose to do something like:

  if (bumpity) fprintf(__FILE__, "%s", "dinglewump");

...the proposed definition would be a disaster.  I doubt that inlining
is a material performance benefit here; siglongjmp() can't be all that
cheap, and the error path shouldn't be all that frequent.

Instead of making ParseConfigFp responsible for restoring
GUC_flex_fatal_jmp after calling anything that might recursively call
ParseConfigFp, I think it would make more sense to define it to stash
away the previous value and restore it on exit.  That way it wouldn't
need to know which of the things that it calls might in turn
recursively call it, which seems likely to reduce the chances of
present or future bugs.  A few comments about whichever way we go with
it seem like a good idea, too.

--=20
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company