| From: | Michael Paquier <michael(at)paquier(dot)xyz> |
|---|---|
| To: | Anton Voloshin <a(dot)voloshin(at)postgrespro(dot)ru> |
| Cc: | Álvaro Herrera <alvherre(at)kurilemu(dot)de>, Euler Taveira <euler(at)eulerto(dot)com>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: NULL pointer dereference in syslogger with load_libraries() and -DEXEC_BACKEND at startup |
| Date: | 2026-07-25 03:54:03 |
| Message-ID: | amQzWzZTFop7AktH@paquier.xyz |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Thu, Jul 16, 2026 at 05:34:54PM +0300, Anton Voloshin wrote:
> Not a problem per se, rather a small inconsistency.
> Since commit
> 8ec569479fc Apply PGDLLIMPORT markings broadly. (2022-04-08) <Robert Haas>
> it seems to be a rule to mark all variables in .h files as PGDLLIMPORT.
> Perhaps, except frontend-only ones.
Coming back to this part of the thread..
Not marking syslogger_setup_done with PGDLLIMPORT is a mistake, or it
would mean that modules based on the elog hook, that care about the
timing of the redirection or the syslogger setup, would fail to build
on WIN32 when trying to use the flag. My jsonlog would hypothetically
fail, though I've never built it on WIN32 (not planning to support or
do so, but others people on this planet are feel to try it).
> As for now,
> $ find src/include -type f -print0 | xargs -0 src/tools/mark_pgdllimport.pl
> src/include/common/logging.h: adding 1 PGDLLIMPORT markers
> src/include/postmaster/syslogger.h: adding 1 PGDLLIMPORT markers
>
> modifies only two lines in two files. And common/logging.h is a
> frontend-only, so it's a natural exception. So I think there is no need
> to add another exception without explicit reasons, even though you are
> right and it's hard for me to think of a realistic need to refer to that
> variable from an extension.
Confirmed. Running another flavor, as of this command, I can see the
same output:
$ src/tools/mark_pgdllimport.pl $(git ls-files src/include/)
src/include/common/logging.h: adding 1 PGDLLIMPORT markers
src/include/postmaster/syslogger.h: adding 1 PGDLLIMPORT markers
Not touching logging.h is intentional, the new syslogger_setup_done
should be marked with PGDLLIMPORT. That's the only hole currently in
the tree.
I'll make that happen down to v19 if there are no objections.
--
Michael
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Amit Kapila | 2026-07-25 04:31:50 | Re: [DESIGN] Soft DROP TABLE, recoverable drops for PostgreSQL |
| Previous Message | Chao Li | 2026-07-25 02:29:18 | Re: Fix race in background worker termination for database |