On Mon, May 9, 2022 at 3:15 AM Michael Paquier <mich...@paquier.xyz> wrote: > On Fri, May 06, 2022 at 04:49:24PM -0700, Andres Freund wrote: > > Just noticed that > > extern sigset_t UnBlockSig, > > BlockSig, > > StartupBlockSig; > > are unadorned. > > mark_pgdllimport.pl is not able to capture this part of the change > because of this logic, where we assume that the header line has to > finish with a semicolon: > # Variable declarations should end in a semicolon. If we see an > # opening parenthesis, it's probably a function declaration. > $needs_pgdllimport = 0 if $stripped_line !~ /;$/ || > $stripped_line =~ /\(/; > > It is quite common to define one variable per line, so I would not put > the blame on the script and just update pqsignal.h. And it is common > to finish lines with a comma for function declarations..
Right. I didn't try to equip the script with a real C parser. Just enough to catch our typical style - which those declarations aren't. > > There's also a number of variables that are only declared in .c files that > > !windows can still access. Some likely aren't worth caring about, but some > > others are underlying GUCs, so we probably do? E.g. > > CommitDelay > > CommitSiblings > > default_tablespace > > ignore_checksum_failure > > ignore_invalid_pages > > Log_disconnections > > ssl_renegotiation_limit > > temp_tablespaces > > wal_level_options > > These are indeed declared in .c files. So you mean that we'd better > declare them in headers and mark them as PGDLLIMPORT? I am not sure > if that's worth the addition, nobody has asked for these to be > available yet, AFAIK. Completely agree. The agreed-upon charter was to adjust the header files, not move any declarations into header files. I'm not against doing that; I think it's good for extensions to have access to GUC values. But it wasn't the mission. > > Also noticed that the bbsink_ops variables are const, instead of static > > const, > > was that intentional? > > Yep, that looks like an error. > > While on it, I think that it would be a good idea to document in the > script that we need pass down a list of header files as arguments to > rewrite them. Either of you please feel free to change these things, at least as far as I'm concerned. -- Robert Haas EDB: http://www.enterprisedb.com
