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 *directory; cfp *dataFH; /* currently open data file */ + char dataFName[MAXPGPATH]; /* file name for dataFH, for error + * message use only */ cfp *blobsTocFH; /* file handle for blobs.toc */ + char blobsTocFName[MAXPGPATH]; /* file name for blobs.toc, for + * error message use only */ + ParallelState *pstate; /* for parallel backup / restore */ } lclContext; @@ -207,6 +212,7 @@ InitArchiveFmt_Directory(ArchiveHandle *AH) fatal("could not open input file \"%s\": %m", fname); ctx->dataFH = tocFH; + strlcpy(ctx->dataFName, fname, MAXPGPATH); /* * The TOC of a directory format dump shares the format code of the @@ -218,9 +224,9 @@ InitArchiveFmt_Directory(ArchiveHandle *AH) ReadToc(AH); /* Nothing else in the file, so close it again... */ - if (cfclose(tocFH) != 0) - fatal("could not close TOC file: %m"); + cfclose(tocFH, "TOC file", fname); ctx->dataFH = NULL; + ctx->dataFName[0] = 0; } } @@ -369,9 +375,11 @@ _EndData(ArchiveHandle *AH, TocEntry *te) lclContext *ctx = (lclContext *) AH->formatData; /* Close the file */ - cfclose(ctx->dataFH); + Assert(AH->mode == archModeWrite); + cfclose(ctx->dataFH, "output file", ctx->dataFName); ctx->dataFH = NULL; + ctx->dataFName[0] = 0; } /* @@ -402,8 +410,8 @@ _PrintFileData(ArchiveHandle *AH, char *filename) } free(buf); - if (cfclose(cfp) != 0) - fatal("could not close data file \"%s\": %m", filename); + + cfclose(cfp, "data file", filename); } /* @@ -466,11 +474,10 @@ _LoadBlobs(ArchiveHandle *AH) fatal("error reading large object TOC file \"%s\"", fname); - if (cfclose(ctx->blobsTocFH) != 0) - fatal("could not close large object TOC file \"%s\": %m", - fname); + cfclose(ctx->blobsTocFH, "large object TOC file", fname); ctx->blobsTocFH = NULL; + ctx->blobsTocFName[0] = 0; EndRestoreBlobs(AH); } @@ -584,6 +591,7 @@ _CloseArchive(ArchiveHandle *AH) if (tocFH == NULL) fatal("could not open output file \"%s\": %m", fname); ctx->dataFH = tocFH; + strlcpy(ctx->dataFName, fname, MAXPGPATH); /* * Write 'tar' in the format field of the toc.dat file. The directory @@ -594,8 +602,9 @@ _CloseArchive(ArchiveHandle *AH) WriteHead(AH); AH->format = archDirectory; WriteToc(AH); - if (cfclose(tocFH) != 0) - fatal("could not close TOC file: %m"); + cfclose(tocFH, "TOC file", fname); + ctx->dataFH = NULL; + ctx->dataFName[0] = 0; WriteDataChunks(AH, ctx->pstate); ParallelBackupEnd(AH, ctx->pstate); @@ -646,6 +655,8 @@ _StartBlobs(ArchiveHandle *AH, TocEntry *te) ctx->blobsTocFH = cfopen_write(fname, "ab", 0); if (ctx->blobsTocFH == NULL) fatal("could not open output file \"%s\": %m", fname); + + strlcpy(ctx->blobsTocFName, fname, MAXPGPATH); } /* @@ -680,8 +691,9 @@ _EndBlob(ArchiveHandle *AH, TocEntry *te, Oid oid) int len; /* Close the BLOB data file itself */ - cfclose(ctx->dataFH); + cfclose(ctx->dataFH, "BLOB data file", ctx->dataFName); ctx->dataFH = NULL; + ctx->dataFName[0] = 0; /* register the blob in blobs.toc */ len = snprintf(buf, sizeof(buf), "%u blob_%u.dat\n", oid, oid); @@ -699,8 +711,9 @@ _EndBlobs(ArchiveHandle *AH, TocEntry *te) { lclContext *ctx = (lclContext *) AH->formatData; - cfclose(ctx->blobsTocFH); + cfclose(ctx->blobsTocFH, "large object TOC file", ctx->blobsTocFName); ctx->blobsTocFH = NULL; + ctx->blobsTocFName[0] = 0; } /* -- 2.18.4