Re: No error checking when reading from file using zstd in pg_dump

> On 25 Jun 2025, at 17:58, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> I looked over this patchset briefly, and found a couple of nits:

Thanks for looking!

> 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.

Instead of this I changed the write_func signature to return the number of
bytes written as size_t. The API is loosely modelled around the libc stream API so
going to void seemed less justifiable than size_t, even if the actual
returnvalue is fairly useless due to erroring out via pg_fatal.

> 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.

Doh, of course, fixed in the attached.

> 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.

I went over all the fwrite callsites *in this patchset* and made sure they
follow the pattern. There are a number of more fwrite calls in pg_dump (and
elsewhere) which might need the same treatment but I left those for a separate
patch to keep this focused on the compression API.

--
Daniel Gustafsson