Re: No error checking when reading from file using zstd in pg_dump - Mailing list pgsql-hackers

From Tom Lane
Subject Re: No error checking when reading from file using zstd in pg_dump
Date
Msg-id 1296195.1750867134@sss.pgh.pa.us
Whole thread Raw
In response to Re: No error checking when reading from file using zstd in pg_dump  (Evgeniy Gorbanev <gorbanyoves@basealt.ru>)
List 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



pgsql-hackers by date:

Previous
From: Álvaro Herrera
Date:
Subject: Re: pg_dump misses comments on NOT NULL constraints
Next
From: Tom Lane
Date:
Subject: Re: Unnecessary scan from non-overlapping range predicates