Re: [patch] pg_dump/pg_restore zerror() and strerror() mishap

From: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
To: Kunshchikov Vladimir <Vladimir(dot)Kunshchikov(at)infotecs(dot)ru>
Cc: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [patch] pg_dump/pg_restore zerror() and strerror() mishap
Date: 2017-08-02 22:36:00
Message-ID: 20170802223600.qoq34k24rebyawxq@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Kunshchikov Vladimir wrote:
> Hello Alvaro,
>
> here goes v4 version: removed unused header.
>
> Compilation of this code snippet with -Wall -Wexter -std=c89 doesn't produce any warnings.

Great, thanks! I have pushed this to all branches since 9.4. Would you
please give it a look? Please let me know if you find any problems
(particularly since you seem to have a test rig to verify it on
corrupted files).

I noticed that with this patch we no longer use WRITE_ERROR_EXIT in
certain cases but instead do the printing/exiting directly. I think
that's fine, but it would be neater to improve the WRITE_ERROR_EXIT
macro so that it takes the cfp as an argument, and then the macro is in
charge of calling get_cfp_error. But then I noticed that it wasn't very
easy to improve things that way. I also noticed that the usage of mixed
compressed/uncompressed file pointers in pg_dump is not very consistent,
and it would take rather a lot of effort to clean up. So I gave up for
now, particularly as a backpatchable bugfix. If you're interested in
mop-up work, I think we can improve pg_dump some more there.

At least, I think most common cases should correctly report any zlib
problems now. Thanks for the patch!

--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2017-08-02 22:50:12 Re: Parallel tuplesort (for parallel B-Tree index creation)
Previous Message Alvaro Herrera 2017-08-02 22:28:48 pgsql: Fix pg_dump's errno checking for zlib I/O