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

2017-07-28 Thread Vladimir Kunschikov
I haven't seen that but can't guarantee that such case does not exist

28 июля 2017 г. 9:19 PM пользователь "Alvaro Herrera" <
alvhe...@2ndquadrant.com> написал:

> Vladimir Kunschikov wrote:
> > >This "maxlen" business and the fallback error message are
> > >strange.  We have roughly equivalent code in pg_basebackup.c
> > >which has been working since 2011
> > >Perhaps you can drop the memchr/fallback tricks and adopt the
> > >pg_basebackup coding?  Or is there a specific reason to have
> > >the memchr check?
> >
> > Ofcourse that tricks can be dropped, function will be much prettier.
> > 'Tricks' were made to pass some strict internal tests.
>
> Well, if there are cases which cause zlib to return a message that
> causes a crash when printing it or some other problem, then let's use
> your version both in this new code and in pg_basebackup.  Do these cases
> really exist?
>
> > Initially I used exactly that function from pg_basebackup.c:
> > https://github.com/kunschikov/postgres/commit/
> 15e9fda6df51cf17c0b0a4f201ee0f93cf258de9#diff-
> 98e3f8ce5d6e87950dd66e4c8bdedb21R713
> > It was rewritten for the sake of somewhat exaggerated security.
> > Version #5 in attachment.
>
> I'd rather have all the security we can get, so before dropping those
> protections, let's make sure they are useless.
>
> --
> Álvaro Herrerahttps://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>


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

2017-07-28 Thread Vladimir Kunschikov
>This "maxlen" business and the fallback error message are
>strange.  We have roughly equivalent code in pg_basebackup.c
>which has been working since 2011
>Perhaps you can drop the memchr/fallback tricks and adopt the
>pg_basebackup coding?  Or is there a specific reason to have
>the memchr check?

Ofcourse that tricks can be dropped, function will be much prettier.
'Tricks' were made to pass some strict internal tests.
Initially I used exactly that function from pg_basebackup.c:
https://github.com/kunschikov/postgres/commit/15e9fda6df51cf17c0b0a4f201ee0f93cf258de9#diff-98e3f8ce5d6e87950dd66e4c8bdedb21R713
It was rewritten for the sake of somewhat exaggerated security.
Version #5 in attachment.

--
Regards,
Vladimir Kunschikov
diff --git a/src/bin/pg_dump/compress_io.c b/src/bin/pg_dump/compress_io.c
index 991fe11..b8af1bc 100644
--- a/src/bin/pg_dump/compress_io.c
+++ b/src/bin/pg_dump/compress_io.c
@@ -709,5 +709,19 @@ hasSuffix(const char *filename, const char *suffix)
   suffix,
   suffixlen) == 0;
 }
+#endif
+
+const char *
+get_cfp_error(cfp* fp)
+{
+#ifdef HAVE_LIBZ
+	if(fp->compressedfp){
+		int errnum;
+		const char *errmsg = gzerror(fp->compressedfp, );
 
+		if( errnum != Z_ERRNO)
+			return errmsg;
+	}
 #endif
+	return strerror(errno);
+}
diff --git a/src/bin/pg_dump/compress_io.h b/src/bin/pg_dump/compress_io.h
index 8f2e752..06b3762 100644
--- a/src/bin/pg_dump/compress_io.h
+++ b/src/bin/pg_dump/compress_io.h
@@ -65,5 +65,6 @@ extern int	cfgetc(cfp *fp);
 extern char *cfgets(cfp *fp, char *buf, int len);
 extern int	cfclose(cfp *fp);
 extern int	cfeof(cfp *fp);
+extern const char * get_cfp_error(cfp *fp);
 
 #endif
diff --git a/src/bin/pg_dump/pg_backup_directory.c b/src/bin/pg_dump/pg_backup_directory.c
index 79922da..e9c26bc 100644
--- a/src/bin/pg_dump/pg_backup_directory.c
+++ b/src/bin/pg_dump/pg_backup_directory.c
@@ -352,7 +352,7 @@ _WriteData(ArchiveHandle *AH, const void *data, size_t dLen)
 	lclContext *ctx = (lclContext *) AH->formatData;
 
 	if (dLen > 0 && cfwrite(data, dLen, ctx->dataFH) != dLen)
-		WRITE_ERROR_EXIT;
+		exit_horribly(modulename, "could not write to output file: %s\n", get_cfp_error(ctx->dataFH));
 
 	return;
 }
@@ -490,7 +490,7 @@ _WriteByte(ArchiveHandle *AH, const int i)
 	lclContext *ctx = (lclContext *) AH->formatData;
 
 	if (cfwrite(, 1, ctx->dataFH) != 1)
-		WRITE_ERROR_EXIT;
+		exit_horribly(modulename, "could not write to output file: %s\n", get_cfp_error(ctx->dataFH));
 
 	return 1;
 }
@@ -519,7 +519,7 @@ _WriteBuf(ArchiveHandle *AH, const void *buf, size_t len)
 	lclContext *ctx = (lclContext *) AH->formatData;
 
 	if (cfwrite(buf, len, ctx->dataFH) != len)
-		WRITE_ERROR_EXIT;
+		exit_horribly(modulename, "could not write to output file: %s\n", get_cfp_error(ctx->dataFH));
 
 	return;
 }
diff --git a/src/bin/pg_dump/pg_backup_tar.c b/src/bin/pg_dump/pg_backup_tar.c
index f839712..b35d3fc 100644
--- a/src/bin/pg_dump/pg_backup_tar.c
+++ b/src/bin/pg_dump/pg_backup_tar.c
@@ -555,8 +555,12 @@ _tarReadRaw(ArchiveHandle *AH, void *buf, size_t len, TAR_MEMBER *th, FILE *fh)
 			{
 res = GZREAD(&((char *) buf)[used], 1, len, th->zFH);
 if (res != len && !GZEOF(th->zFH))
+{
+	int errnum;
+	const char *errmsg = gzerror(th->zFH, );
 	exit_horribly(modulename,
-  "could not read from input file: %s\n", strerror(errno));
+  "could not read from input file: %s\n", errnum == Z_ERRNO? strerror(errno) : errmsg);
+}
 			}
 			else
 			{

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