From: Kunshchikov Vladimir <Vladimir.Kunshchikov@infotecs.ru>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
CC: "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Subject: Re: [patch] pg_dump/pg_restore zerror() and strerror()
 mishap
Thread-Topic: [HACKERS] [patch] pg_dump/pg_restore zerror() and strerror()
 mishap
Thread-Index: AQHS6zMy2h4IVOLogEaPttzhjwdFBqJlGr8AgAFVnqs=
Date: Wed, 26 Jul 2017 15:32:33 +0000
Message-ID: <1501082204231.82652@infotecs.ru>
References: 
 <1498120508308.9826@infotecs.ru>,<20170725220234.x2fprth7taeb6oc3@alvherre.pgsql>
In-Reply-To: <20170725220234.x2fprth7taeb6oc3@alvherre.pgsql>
Accept-Language: en-US
Content-Language: en-US
Content-Type: multipart/mixed; boundary="_002_150108220423182652infotecsru_"
MIME-Version: 1.0
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org

--_002_150108220423182652infotecsru_
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable

 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() m=
ishap

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.

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

--_002_150108220423182652infotecsru_
Content-Type: text/x-patch; name="strerror-gzerror-zlib-v2.patch"
Content-Description: strerror-gzerror-zlib-v2.patch
Content-Disposition: attachment; filename="strerror-gzerror-zlib-v2.patch";
	size=3153; creation-date="Wed, 26 Jul 2017 15:32:29 GMT";
	modification-date="Wed, 26 Jul 2017 15:32:29 GMT"
Content-Transfer-Encoding: base64

ZGlmZiAtLWdpdCBhL3NyYy9iaW4vcGdfZHVtcC9jb21wcmVzc19pby5jIGIv
c3JjL2Jpbi9wZ19kdW1wL2NvbXByZXNzX2lvLmMKaW5kZXggOTkxZmUxMWU4
YS4uNjc5MDhhMDY5YyAxMDA2NDQKLS0tIGEvc3JjL2Jpbi9wZ19kdW1wL2Nv
bXByZXNzX2lvLmMKKysrIGIvc3JjL2Jpbi9wZ19kdW1wL2NvbXByZXNzX2lv
LmMKQEAgLTcwOSw1ICs3MDksMjUgQEAgaGFzU3VmZml4KGNvbnN0IGNoYXIg
KmZpbGVuYW1lLCBjb25zdCBjaGFyICpzdWZmaXgpCiAJCQkJICBzdWZmaXgs
CiAJCQkJICBzdWZmaXhsZW4pID09IDA7CiB9CisjZW5kaWYKIAorY29uc3Qg
Y2hhciAqCitnZXRfY2ZwX2Vycm9yKGNmcCogZnApCit7CisjaWZkZWYgSEFW
RV9MSUJaCisJaWYoIWZwLT5jb21wcmVzc2VkZnApCisJCXJldHVybiBzdHJl
cnJvcihlcnJubyk7CisKKwlpbnQgZXJybnVtOworCQorCXN0YXRpYyBjb25z
dCBjaGFyIGZhbGxiYWNrW10gPSAiWmxpYiBlcnJvciI7CisJY29uc3QgaW50
IG1heGxlbiA9IDI1NTsKKwljb25zdCBjaGFyICplcnJtc2cgPSBnemVycm9y
KGZwLT5jb21wcmVzc2VkZnAsICZlcnJudW0pOworCWlmKCFlcnJtc2cgfHwg
IW1lbWNocihlcnJtc2csIDAsIG1heGxlbikpCisJCWVycm1zZyA9IGZhbGxi
YWNrOworCisJcmV0dXJuIGVycm51bSA9PSBaX0VSUk5PID8gc3RyZXJyb3Io
ZXJybm8pIDogZXJybXNnOworI2Vsc2UKKwlyZXR1cm4gc3RyZXJyb3IoZXJy
bm8pOwogI2VuZGlmCit9CmRpZmYgLS1naXQgYS9zcmMvYmluL3BnX2R1bXAv
Y29tcHJlc3NfaW8uaCBiL3NyYy9iaW4vcGdfZHVtcC9jb21wcmVzc19pby5o
CmluZGV4IDhmMmU3NTJjYmEuLjA2YjM3NjIyMzMgMTAwNjQ0Ci0tLSBhL3Ny
Yy9iaW4vcGdfZHVtcC9jb21wcmVzc19pby5oCisrKyBiL3NyYy9iaW4vcGdf
ZHVtcC9jb21wcmVzc19pby5oCkBAIC02NSw1ICs2NSw2IEBAIGV4dGVybiBp
bnQJY2ZnZXRjKGNmcCAqZnApOwogZXh0ZXJuIGNoYXIgKmNmZ2V0cyhjZnAg
KmZwLCBjaGFyICpidWYsIGludCBsZW4pOwogZXh0ZXJuIGludAljZmNsb3Nl
KGNmcCAqZnApOwogZXh0ZXJuIGludAljZmVvZihjZnAgKmZwKTsKK2V4dGVy
biBjb25zdCBjaGFyICogZ2V0X2NmcF9lcnJvcihjZnAgKmZwKTsKIAogI2Vu
ZGlmCmRpZmYgLS1naXQgYS9zcmMvYmluL3BnX2R1bXAvcGdfYmFja3VwX2Rp
cmVjdG9yeS5jIGIvc3JjL2Jpbi9wZ19kdW1wL3BnX2JhY2t1cF9kaXJlY3Rv
cnkuYwppbmRleCA3OTkyMmRhOGJhLi5lOWMyNmJjNTcxIDEwMDY0NAotLS0g
YS9zcmMvYmluL3BnX2R1bXAvcGdfYmFja3VwX2RpcmVjdG9yeS5jCisrKyBi
L3NyYy9iaW4vcGdfZHVtcC9wZ19iYWNrdXBfZGlyZWN0b3J5LmMKQEAgLTM1
Miw3ICszNTIsNyBAQCBfV3JpdGVEYXRhKEFyY2hpdmVIYW5kbGUgKkFILCBj
b25zdCB2b2lkICpkYXRhLCBzaXplX3QgZExlbikKIAlsY2xDb250ZXh0ICpj
dHggPSAobGNsQ29udGV4dCAqKSBBSC0+Zm9ybWF0RGF0YTsKIAogCWlmIChk
TGVuID4gMCAmJiBjZndyaXRlKGRhdGEsIGRMZW4sIGN0eC0+ZGF0YUZIKSAh
PSBkTGVuKQotCQlXUklURV9FUlJPUl9FWElUOworCQlleGl0X2hvcnJpYmx5
KG1vZHVsZW5hbWUsICJjb3VsZCBub3Qgd3JpdGUgdG8gb3V0cHV0IGZpbGU6
ICVzXG4iLCBnZXRfY2ZwX2Vycm9yKGN0eC0+ZGF0YUZIKSk7CiAKIAlyZXR1
cm47CiB9CkBAIC00OTAsNyArNDkwLDcgQEAgX1dyaXRlQnl0ZShBcmNoaXZl
SGFuZGxlICpBSCwgY29uc3QgaW50IGkpCiAJbGNsQ29udGV4dCAqY3R4ID0g
KGxjbENvbnRleHQgKikgQUgtPmZvcm1hdERhdGE7CiAKIAlpZiAoY2Z3cml0
ZSgmYywgMSwgY3R4LT5kYXRhRkgpICE9IDEpCi0JCVdSSVRFX0VSUk9SX0VY
SVQ7CisJCWV4aXRfaG9ycmlibHkobW9kdWxlbmFtZSwgImNvdWxkIG5vdCB3
cml0ZSB0byBvdXRwdXQgZmlsZTogJXNcbiIsIGdldF9jZnBfZXJyb3IoY3R4
LT5kYXRhRkgpKTsKIAogCXJldHVybiAxOwogfQpAQCAtNTE5LDcgKzUxOSw3
IEBAIF9Xcml0ZUJ1ZihBcmNoaXZlSGFuZGxlICpBSCwgY29uc3Qgdm9pZCAq
YnVmLCBzaXplX3QgbGVuKQogCWxjbENvbnRleHQgKmN0eCA9IChsY2xDb250
ZXh0ICopIEFILT5mb3JtYXREYXRhOwogCiAJaWYgKGNmd3JpdGUoYnVmLCBs
ZW4sIGN0eC0+ZGF0YUZIKSAhPSBsZW4pCi0JCVdSSVRFX0VSUk9SX0VYSVQ7
CisJCWV4aXRfaG9ycmlibHkobW9kdWxlbmFtZSwgImNvdWxkIG5vdCB3cml0
ZSB0byBvdXRwdXQgZmlsZTogJXNcbiIsIGdldF9jZnBfZXJyb3IoY3R4LT5k
YXRhRkgpKTsKIAogCXJldHVybjsKIH0KZGlmZiAtLWdpdCBhL3NyYy9iaW4v
cGdfZHVtcC9wZ19iYWNrdXBfdGFyLmMgYi9zcmMvYmluL3BnX2R1bXAvcGdf
YmFja3VwX3Rhci5jCmluZGV4IGY4Mzk3MTI5NDUuLmYxN2NlNzhhY2EgMTAw
NjQ0Ci0tLSBhL3NyYy9iaW4vcGdfZHVtcC9wZ19iYWNrdXBfdGFyLmMKKysr
IGIvc3JjL2Jpbi9wZ19kdW1wL3BnX2JhY2t1cF90YXIuYwpAQCAtMzUsNiAr
MzUsNyBAQAogI2luY2x1ZGUgInBndGFyLmgiCiAjaW5jbHVkZSAiY29tbW9u
L2ZpbGVfdXRpbHMuaCIKICNpbmNsdWRlICJmZV91dGlscy9zdHJpbmdfdXRp
bHMuaCIKKyNpbmNsdWRlICJjb21wcmVzc19pby5oIgogCiAjaW5jbHVkZSA8
c3lzL3N0YXQuaD4KICNpbmNsdWRlIDxjdHlwZS5oPgpAQCAtNTU1LDggKzU1
NiwxMiBAQCBfdGFyUmVhZFJhdyhBcmNoaXZlSGFuZGxlICpBSCwgdm9pZCAq
YnVmLCBzaXplX3QgbGVuLCBUQVJfTUVNQkVSICp0aCwgRklMRSAqZmgpCiAJ
CQl7CiAJCQkJcmVzID0gR1pSRUFEKCYoKGNoYXIgKikgYnVmKVt1c2VkXSwg
MSwgbGVuLCB0aC0+ekZIKTsKIAkJCQlpZiAocmVzICE9IGxlbiAmJiAhR1pF
T0YodGgtPnpGSCkpCisJCQkJeworCQkJCQlpbnQgZXJybnVtOworCQkJCQlj
b25zdCBjaGFyICplcnJtc2cgPSBnemVycm9yKHRoLT56RkgsICZlcnJudW0p
OwogCQkJCQlleGl0X2hvcnJpYmx5KG1vZHVsZW5hbWUsCi0JCQkJCQkJCSAg
ImNvdWxkIG5vdCByZWFkIGZyb20gaW5wdXQgZmlsZTogJXNcbiIsIHN0cmVy
cm9yKGVycm5vKSk7CisJCQkJCQkJCSAgImNvdWxkIG5vdCByZWFkIGZyb20g
aW5wdXQgZmlsZTogJXNcbiIsIGVycm51bSA9PSBaX0VSUk5PPyBzdHJlcnJv
cihlcnJubykgOiBlcnJtc2cpOworCQkJCX0KIAkJCX0KIAkJCWVsc2UKIAkJ
CXsK

--_002_150108220423182652infotecsru_
Content-Type: text/plain
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0


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

--_002_150108220423182652infotecsru_--