pgsql: Improve type handling in pg_dump's compress file API - Mailing list pgsql-committers

From Tomas Vondra
Subject pgsql: Improve type handling in pg_dump's compress file API
Date
Msg-id E1pfOGY-0050rh-9g@gemulon.postgresql.org
Whole thread Raw
List pgsql-committers
Improve type handling in pg_dump's compress file API

After 0da243fed0 got committed, we've received a report about a compiler
warning, related to the new LZ4File_gets() function:

  compress_lz4.c: In function 'LZ4File_gets':
  compress_lz4.c:492:19: warning: comparison of unsigned expression in
                                  '< 0' is always false [-Wtype-limits]
    492 |         if (dsize < 0)

The reason is very simple - dsize is declared as size_t, which is an
unsigned integer, and thus the check is pointless and we might fail to
notice an error in some cases (or fail in a strange way a bit later).

The warning could have been silenced by simply changing the type, but we
realized the API mostly assumes all the libraries use the same types and
report errors the same way (e.g. by returning 0 and/or negative value).

But we can't make this assumption - the gzip/lz4 libraries already
disagree on some of this, and even if they did a library added in the
future might not.

The right solution is to define what the API does, and translate the
library-specific behavior in consistent way (so that the internal errors
are not exposed to users of our compression API). So this adjusts the
data types in a couple places, so that we don't miss library errors, and
simplifies and unifies the error reporting to simply return true/false
(instead of e.g. size_t).

While at it, make sure LZ4File_open_write() does not clobber errno in
case open_func() fails.

Author: Georgios Kokolatos
Reported-by: Alexander Lakhin
Reviewed-by: Tomas Vondra, Justin Pryzby
Discussion: https://postgr.es/m/33496f7c-3449-1426-d568-63f6bca2ac1f@gmail.com

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/d3b57755e60c2d3cfeddbc733c6168419d874414

Modified Files
--------------
src/bin/pg_dump/compress_gzip.c       | 37 ++++++++-------
src/bin/pg_dump/compress_io.c         |  8 ++--
src/bin/pg_dump/compress_io.h         | 38 ++++++++++++----
src/bin/pg_dump/compress_lz4.c        | 85 +++++++++++++++++++----------------
src/bin/pg_dump/compress_none.c       | 41 ++++++++++-------
src/bin/pg_dump/pg_backup_archiver.c  | 18 +++-----
src/bin/pg_dump/pg_backup_directory.c | 36 +++++++--------
7 files changed, 148 insertions(+), 115 deletions(-)


pgsql-committers by date:

Previous
From: Jeff Davis
Date:
Subject: pgsql: Wrap ICU ucol_open().
Next
From: Tom Lane
Date:
Subject: pgsql: Add missing "-I." flag when building pg_bsd_indent.