Re: [patch] pg_dump/pg_restore zerror() and strerror() mishap
Kunshchikov Vladimir <vladimir.kunshchikov@infotecs.ru>
From: Kunshchikov Vladimir <Vladimir.Kunshchikov@infotecs.ru>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2017-07-26T15:32:33Z
Lists: pgsql-hackers
Attachments
- strerror-gzerror-zlib-v2.patch (text/x-patch) patch v2
Hello Alvaro, thanks for the feedback, fixed all of your points. Attached new version of patch. -- Best regards, Vladimir Kunschikov Lead software developer IDS project InfoTeCS JSC ________________________________________ From: Alvaro Herrera <alvherre@2ndquadrant.com> Sent: Wednesday, July 26, 2017 1:02 AM To: Kunshchikov Vladimir Cc: pgsql-hackers@postgresql.org Subject: Re: [HACKERS] [patch] pg_dump/pg_restore zerror() and strerror() mishap Kunshchikov Vladimir wrote: > Errors should be like this: > pg_restore: [compress_io] could not read from input file: d3/2811.dat.gz: invalid distance too far back > > Attached small fix for this issue. After looking at this patch, I don't like it very much. In particular, I don't like the way you've handled the WRITE_ERROR_EXIT macro in pg_backup_directory.c by undef'ing the existing one and creating it anew. The complete and correct definition should reside in one place (pg_backup_archiver.h), instead of being split in two and being defined differently. Another point is that you broke the comment on the definition of struct cfp "this is opaque to callers" by moving it to the header file precisely with the point of making it transparent to callers. We need some better idea there. I think it can be done by making compress_io.c responsible of handing over the error message through some new function (something very simple like "if compressedfp then return get_gz_error else strerror" should suffice). Also, I needed to rebase your patch over a recent pgindent run, though that's a pretty small change. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Commits
-
Fix pg_dump's errno checking for zlib I/O
- 4d57e8381677 10.0 landed
- f3142c027212 9.5.8 landed
- 060393f2a136 9.6.4 landed
- 330e68734b82 9.4.13 landed
-
Add pg_basebackup tool for streaming base backups
- 048d148fe631 9.1.0 cited