Re: No error checking when reading from file using zstd in pg_dump
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Daniel Gustafsson <daniel@yesql.se>
Cc: Tomas Vondra <tomas@vondra.me>, Evgeniy Gorbanev <gorbanyoves@basealt.ru>,
pgsql-hackers@lists.postgresql.org
Date: 2025-06-25T15:58:54Z
Lists: pgsql-hackers
Daniel Gustafsson <daniel@yesql.se> writes: > I spent a little bit of time reading over all the implementations and cross > referencing the API for conformity, and came up with the attached. The 0001 > patch is the one from upstream, and each subsequent commit is fixing one > function for all the implementations. Before pushing it should all be squashed > into a single commit IMHO. Thanks for tackling this! I looked over this patchset briefly, and found a couple of nits: v5-0002, in compress_io.h: + * Returns true on success and throws error for all error conditions. It doesn't return true anymore. Should be more like + * Returns nothing. Exits via pg_fatal for all error conditions. In LZ4Stream_write: you dropped the bit about - errno = (errno) ? errno : ENOSPC; but I think that's still necessary: we must assume ENOSPC if fwrite doesn't set errno. Other fwrite callers (write_none, Zstd_write) need this too. v5-0004 has an instance too, in Zstd_close. I did not check to see if other fwrite calls are OK, but it'd be good to verify that they all follow the pattern of presetting errno to 0 and then replacing that with ENOSPC. regards, tom lane
Commits
-
pg_dump: Fix compression API errorhandling
- ec017a305bd4 16.11 landed
- e686010c5b47 19 (unreleased) landed
- 92268b35d04c 17.7 landed
- 8980c724b559 18.0 landed