At Wed, 21 Oct 2020 13:45:15 +0900 (JST), Kyotaro Horiguchi
wrote in
> https://www.postgresql.org/message-id/flat/20200416.181945.759179589924840062.horikyota.ntt%40gmail.com#ed85c5fda64873c45811be4e3027a2ea
>
> Me> Hmm. Sounds reasonable. I'm going to do that. Thanks!
>
> But somehow that haven't happened, I'll come up with a new version.
pg_restore shows the following error instead of "Success" for broken
compressed file.
pg_restore: error: could not close data file "d/3149.dat": zlib error: error
reading or writing compressed file
0001:
cfclose() calls fatal() instead of returning the result to the callers
on error, which isobviously okay for all existing callers that are
handling errors from the function. Other callers ignored the returned
value but we should fatal() on error of the function.
At least for me, gzerror doesn't return a message (specifically,
returns NULL) after gzclose failure so currently cfclose shows its own
messages for erros of gzclose(). Am I missing something?
0002:
cfread has the same code with get_cfp_error() and cfgetc uses
sterror() after gzgetc(). It would be suitable for a separate patch,
but 0002 fixes those bugs. I changed _EndBlob() to show the cause of
an error.
Did not do in this patch:
We could do further small refactoring to remove temporary variables in
pg_backup_directory.c for _StartData(), InitArchiveFmt_Directory,
_LoadBlobs(), _StartBlobs() and _CloseArchive(), but I left them as is
for the ease of back-patching.
Now that we have the file name in the context variable so we could
show the file name in all error messages, but that change was large
and there's a part where that change is a bit more complex so I didn't
do that.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
>From 15190e795dbc4a6245d96d37ee561624769ab7ae Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi
Date: Wed, 21 Oct 2020 11:50:02 +0900
Subject: [PATCH 1/2] Fix error handling for file close of pg_dump
Errors from cfclose is handled as if it were fclose, but the function
returned errors from gzclose and the callers show incorrect error
messages like "could not close: Success". Fix that behavior by moving
the error handling code into cfclose.
---
src/bin/pg_dump/compress_io.c | 38 +++
src/bin/pg_dump/compress_io.h | 2 +-
src/bin/pg_dump/pg_backup_directory.c | 37 +-
3 files changed, 59 insertions(+), 18 deletions(-)
diff --git a/src/bin/pg_dump/compress_io.c b/src/bin/pg_dump/compress_io.c
index 1417401086..28d45fd299 100644
--- a/src/bin/pg_dump/compress_io.c
+++ b/src/bin/pg_dump/compress_io.c
@@ -645,31 +645,59 @@ cfgets(cfp *fp, char *buf, int len)
return fgets(buf, len, fp->uncompressedfp);
}
-int
-cfclose(cfp *fp)
+/*
+ * cfclose close the stream
+ *
+ * Returns if successfully closed the cfp. End the process with fatal() on
+ * error. filedesc is a label that identifies the file type and filename is the
+ * name of the file, both of which are used for error reporting.
+ */
+void
+cfclose(cfp *fp, const char *filedesc, const char *filename)
{
int result;
if (fp == NULL)
{
errno = EBADF;
- return EOF;
+
+ fatal("could not close %s \"%s\": %m", filedesc, filename);
}
#ifdef HAVE_LIBZ
if (fp->compressedfp)
{
result = gzclose(fp->compressedfp);
fp->compressedfp = NULL;
+
+ switch (result)
+ {
+ case Z_OK:
+break;
+
+ case Z_STREAM_ERROR:
+fatal("could not close %s \"%s\": zlib error: invalid parameter",
+ filedesc, filename);
+ case Z_ERRNO:
+fatal("could not close %s \"%s\": %m", filedesc, filename);
+ case Z_BUF_ERROR:
+fatal("could not close %s \"%s\": zlib error: error reading or writing compressed file",
+ filedesc, filename);
+ default:
+fatal("could not close %s \"%s\": zlib error: unknown gzclose error: %d",
+ filedesc, filename, result);
+ }
}
else
#endif
{
result = fclose(fp->uncompressedfp);
fp->uncompressedfp = NULL;
+
+ if (result != 0)
+ fatal("could not close %s \"%s\": %m", filedesc, filename);
}
+
free_keep_errno(fp);
-
- return result;
}
int
diff --git a/src/bin/pg_dump/compress_io.h b/src/bin/pg_dump/compress_io.h
index d2e6e1b854..97986636c7 100644
--- a/src/bin/pg_dump/compress_io.h
+++ b/src/bin/pg_dump/compress_io.h
@@ -63,7 +63,7 @@ extern int cfread(void *ptr, int size, cfp *fp);
extern int cfwrite(const void *ptr, int size, cfp *fp);
extern int cfgetc(cfp *fp);
extern char *cfgets(cfp *fp, char *buf, int len);
-extern int cfclose(cfp *fp);
+extern void cfclose(cfp *fp, const char *filedesc, const char *filename);
extern int cfeof(cfp *fp);
extern const char *get_cfp_error(cfp *fp);
diff --git a/src/bin/pg_dump/pg_backup_directory.c b/src/bin/pg_dump/pg_backup_directory.c
index 48fa7cb1a3..4b1cf4a5a3 100644
--- a/src/bin/pg_dump/pg_backup_directory.c
+++ b/src/bin/pg_dump/pg_backup_directory.c
@@ -51,8 +51,13 @@ typedef struct
char
