pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Kunshchikov Vladimir <Vladimir.Kunshchikov@infotecs.ru>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Subject: Re: [patch] pg_dump/pg_restore zerror() and strerror() mishap
Date: Tue, 25 Jul 2017 18:02:34 -0400
Message-ID: <20170725220234.x2fprth7taeb6oc3@alvherre.pgsql> (raw)
In-Reply-To: <1498120508308.9826@infotecs.ru>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



view thread (12+ messages)  latest in thread

Message-ID: <20170725220234.x2fprth7taeb6oc3@alvherre.pgsql>
Permalink:  ../20170725220234.x2fprth7taeb6oc3@alvherre.pgsql/
Also on:    postgresql.org/message-id/20170725220234.x2fprth7taeb6oc3@alvherre.pgsql

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: alvherre@2ndquadrant.com, Vladimir.Kunshchikov@infotecs.ru
  Subject: Re: [patch] pg_dump/pg_restore zerror() and strerror() mishap
  In-Reply-To: <20170725220234.x2fprth7taeb6oc3@alvherre.pgsql>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox