Horton

Thread

Commits

  1. Don't rely on zlib's gzgetc() macro.

  1. gzgetc() is hazardous to your health

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-10-19T04:06:23Z

    In the gift-that-keeps-on-giving department: I noticed a few days ago
while fooling with pg_dump's compression logic that our regression
tests were not reaching any of the getc_func methods.  It turns out
that that's only used when reading a manually-compressed TOC file in
a directory-format dump, and we were not exercising that case.  That
oversight already allowed one undetected bug (cf a239c4a0c), so I put
in 20ec99589 ... and mamba promptly fell over [1].  After a couple of
hours of digging I found that there's a configuration-sensitive bug in
NetBSD's 32-bit build of zlib [2]: if HAVE_UNISTD_H is defined before
including <zlib.h> then the gzgetc() macro ends up thinking that
gzFile_s.pos is 64 bits, while the library thinks it's 32.  Ooops.

This can't be blamed entirely on NetBSD: zconf.h is hoary old code
that tries to deal with a lot of not-very-consistent platforms.
I think it likely that other platforms have similar issues, which
we'd not notice given that (a) we haven't been testing this case and
(b) probably few users are invoking it either and (c) it probably
only fails on 32-bit platforms.

What I think we ought to do about this is get rid of our one usage
of gzgetc(), replacing it with a one-byte gzread() operation.
That's not lovely from a speed perspective, but I don't think that
reading a pg_dump TOC file is really speed-critical.

			regards, tom lane

PS: I've temporarily disabled --enable-tap-tests on mamba so
that it will go back to green.  That's hardly a fix though.

[1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=mamba&dt=2025-10-17%2006%3A09%3A19
[2] https://gnats.netbsd.org/cgi-bin/query-pr-single.pl?number=59711

  2. Re: gzgetc() is hazardous to your health

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-10-19T16:57:16Z

    I wrote:
> What I think we ought to do about this is get rid of our one usage
> of gzgetc(), replacing it with a one-byte gzread() operation.
> That's not lovely from a speed perspective, but I don't think that
> reading a pg_dump TOC file is really speed-critical.

In the light of morning I had a better, or at least easier, idea:
just #undef gzgetc and fall back on the underlying function.
That's at least a little faster than gzread(), too.

			regards, tom lane