Re: ECPG cleanup and fix for clang compile-time problem

Tom Lane <tgl@sss.pgh.pa.us>

From: Tom Lane <tgl@sss.pgh.pa.us>
To: John Naylor <johncnaylorls@gmail.com>
Cc: Peter Eisentraut <peter@eisentraut.org>, "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>, Andres Freund <andres@anarazel.de>, Michael Paquier <michael.paquier@gmail.com>
Date: 2024-10-14T18:25:10Z
Lists: pgsql-hackers

Attachments

John Naylor <johncnaylorls@gmail.com> writes:
>> [v5]

Thanks for reviewing!  I pushed 0001-0005 and 0009, adopting
your suggestions except for

> + /* List a token here if pgc.l assigns to base_yylval.str for it */
> Does pgc.l need to have a similar comment?

That's not a bad suggestion, but I couldn't see any very useful place
to put such a comment.

> I haven't looked closely at 0006 through 0009. One possible concern is
> that the regression tests might not cover very well, but if you can
> get valgrind silent for memory leaks for what they do cover, that's
> certainly a good step.

Attached are rebased and renumbered 0006-0008, mostly to keep the
cfbot happy.  We could actually stop here, if we were feeling lazy,
but now that I've done the work I'm inclined to push forward with
the rest.

The rest is just memory leak removal, and I suspect that nobody really
cares that much about small leakage in the preprocessor: you'd have to
be running some darn big files through it to notice.  FTR, here are
the total leaks reported by valgrind for running the ecpg regression
tests, using code like

$ grep lost: *log | tr -d ',' | awk '{sum += $5} 
END {print sum}'

Before these patches:	25743
after 0003:		59049363
after 0005:		141556		(this is master now)
after 0006(0001):	132633
after 0007(0002):	9087
after 0008(0003):	0

So clearly, 0003 by itself wasn't good enough, but arguably no
real users will notice the extra inefficiency as of HEAD.
Still, I'd kind of like to get 0007 (now 0002) in there, and
I believe 0006 (0001) is a necessary prerequisite to that.

			regards, tom lane

Commits

  1. ecpg: clean up some other assorted memory leaks.

  2. ecpg: fix some memory leakage of data-type-related structures.

  3. ecpg: put all string-valued tokens returned by pgc.l in local storage.

  4. ecpg: fix more minor mishandling of bad input in preprocessor.

  5. ecpg: fix some minor mishandling of bad input in preprocessor.

  6. ecpg: avoid breaking the IDENT precedence level in two.

  7. ecpg: improve preprocessor's memory management.

  8. ecpg: move some functions into a new file ecpg/preproc/util.c.

  9. ecpg: re-implement preprocessor's string management.

  10. ecpg: major cleanup, simplification, and documentation of parse.pl.

  11. ecpg: remove check_rules.pl.

  12. ecpg: clean up documentation of parse.pl, and add more input checking.

  13. Clean up indentation and whitespace inconsistencies in ecpg.

  14. Exclude flex-generated code from coverage testing