Repair various defects in dc212340058b4e7ecfc5a7a81ec50e7a207bf288.

pg_combinebackup had various problems:

* strncpy was used in various places where strlcpy should be used
  instead, to avoid any possibility of the result not being
  \0-terminated.
* scan_for_existing_tablespaces() failed to close the directory,
  and an error when opening the directory was reported with the
  wrong pathname.
* write_reconstructed_file() contained some redundant and therefore
  dead code.
* flush_manifest() didn't check the result of pg_checksum_update()
  as we do in other places, and misused a local pathname variable
  that shouldn't exist at all.

In pg_basebackup, the wrong variable name was used in one place,
due to a copy and paste that was not properly adjusted.

In blkreftable.c, the loop incorrectly doubled chunkno instead of
max_chunks. Fix that. Also remove a nearby assertion per repeated
off-list complaints from Tom Lane.

Per Coverity and subsequent code inspection by me and by Tom Lane.

Discussion: http://postgr.es/m/CA+Tgmobvqqj-DW9F7uUzT-cQqs6wcVb-Xhs=w=hzJnXSE-kRGw@mail.gmail.com

diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index e906d5185c185728d579deb4113128bbb4b6691a..77489af518885b44aafd93342714708002292730 100644 (file)
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -709,7 +709,7 @@ StartLogStreamer(char *startpos, uint32 timeline, char *sysidentifier,
                     PQserverVersion(conn) < MINIMUM_VERSION_FOR_PG_WAL ?
                     "pg_xlog" : "pg_wal");
 
-           if (pg_mkdir_p(statusdir, pg_dir_create_mode) != 0 &&
+           if (pg_mkdir_p(summarydir, pg_dir_create_mode) != 0 &&
                errno != EEXIST)
                pg_fatal("could not create directory \"%s\": %m", summarydir);
        }

diff --git a/src/bin/pg_combinebackup/pg_combinebackup.c b/src/bin/pg_combinebackup/pg_combinebackup.c
index 6027e241f3060acb575c24f323d5be9c5dc8ad96..31ead7f40587fbd177aaf44891b881db3b030338 100644 (file)
--- a/src/bin/pg_combinebackup/pg_combinebackup.c
+++ b/src/bin/pg_combinebackup/pg_combinebackup.c
@@ -773,8 +773,8 @@ process_directory_recursively(Oid tsoid,
     */
    if (relative_path == NULL)
    {
-       strncpy(ifulldir, input_directory, MAXPGPATH);
-       strncpy(ofulldir, output_directory, MAXPGPATH);
+       strlcpy(ifulldir, input_directory, MAXPGPATH);
+       strlcpy(ofulldir, output_directory, MAXPGPATH);
        if (OidIsValid(tsoid))
            snprintf(manifest_prefix, MAXPGPATH, "pg_tblspc/%u/", tsoid);
        else
@@ -855,7 +855,7 @@ process_directory_recursively(Oid tsoid,
 
            /* Append new pathname component to relative path. */
            if (relative_path == NULL)
-               strncpy(new_relative_path, de->d_name, MAXPGPATH);
+               strlcpy(new_relative_path, de->d_name, MAXPGPATH);
            else
                snprintf(new_relative_path, MAXPGPATH, "%s/%s", relative_path,
                         de->d_name);
@@ -1131,7 +1131,7 @@ scan_for_existing_tablespaces(char *pathname, cb_options *opt)
    pg_log_debug("scanning \"%s\"", pg_tblspc);
 
    if ((dir = opendir(pg_tblspc)) == NULL)
-       pg_fatal("could not open directory \"%s\": %m", pathname);
+       pg_fatal("could not open directory \"%s\": %m", pg_tblspc);
 
    while (errno = 0, (de = readdir(dir)) != NULL)
    {
@@ -1203,8 +1203,8 @@ scan_for_existing_tablespaces(char *pathname, cb_options *opt)
            {
                if (strcmp(tsmap->old_dir, link_target) == 0)
                {
-                   strncpy(ts->old_dir, tsmap->old_dir, MAXPGPATH);
-                   strncpy(ts->new_dir, tsmap->new_dir, MAXPGPATH);
+                   strlcpy(ts->old_dir, tsmap->old_dir, MAXPGPATH);
+                   strlcpy(ts->new_dir, tsmap->new_dir, MAXPGPATH);
                    ts->in_place = false;
                    break;
                }
@@ -1238,6 +1238,9 @@ scan_for_existing_tablespaces(char *pathname, cb_options *opt)
        tslist = ts;
    }
 
+   if (closedir(dir) != 0)
+       pg_fatal("could not close directory \"%s\": %m", pg_tblspc);
+
    return tslist;
 }
 

diff --git a/src/bin/pg_combinebackup/reconstruct.c b/src/bin/pg_combinebackup/reconstruct.c
index b835ec363e72fd95b3419b99f1e06d2e88bd6c97..873d3079025b9dc3e8ab332147b8bb58c25ea9d5 100644 (file)
--- a/src/bin/pg_combinebackup/reconstruct.c
+++ b/src/bin/pg_combinebackup/reconstruct.c
@@ -577,13 +577,12 @@ write_reconstructed_file(char *input_filename,
            {
                if (current_block == start_of_range)
                    appendStringInfo(&debug_buf, " %u:%s@" UINT64_FORMAT,
-                                    current_block,
-                                    s == NULL ? "ZERO" : s->filename,
+                                    current_block, s->filename,
                                     (uint64) offsetmap[current_block]);
                else
                    appendStringInfo(&debug_buf, " %u-%u:%s@" UINT64_FORMAT,
                                     start_of_range, current_block,
-                                    s == NULL ? "ZERO" : s->filename,
+                                    s->filename,
                                     (uint64) offsetmap[current_block]);
            }
 

diff --git a/src/bin/pg_combinebackup/write_manifest.c b/src/bin/pg_combinebackup/write_manifest.c
index 608e84f3b459819551d32504e85a2580ef5ba042..01deb82cc9fb1c76806e6057ac4c3279734bb1eb 100644 (file)
--- a/src/bin/pg_combinebackup/write_manifest.c
+++ b/src/bin/pg_combinebackup/write_manifest.c
@@ -241,8 +241,6 @@ escape_json(StringInfo buf, const char *str)
 static void
 flush_manifest(manifest_writer *mwriter)
 {
-   char        pathname[MAXPGPATH];
-
    if (mwriter->fd == -1 &&
        (mwriter->fd = open(mwriter->pathname,
                            O_WRONLY | O_CREAT | O_EXCL | PG_BINARY,
@@ -260,13 +258,15 @@ flush_manifest(manifest_writer *mwriter)
                pg_fatal("could not write \"%s\": %m", mwriter->pathname);
            else
                pg_fatal("could not write file \"%s\": wrote only %d of %d bytes",
-                        pathname, (int) wb, mwriter->buf.len);
+                        mwriter->pathname, (int) wb, mwriter->buf.len);
        }
 
-       if (mwriter->still_checksumming)
+       if (mwriter->still_checksumming &&
            pg_checksum_update(&mwriter->manifest_ctx,
                               (uint8 *) mwriter->buf.data,
-                              mwriter->buf.len);
+                              mwriter->buf.len) < 0)
+           pg_fatal("could not update checksum of file \"%s\"",
+                    mwriter->pathname);
        resetStringInfo(&mwriter->buf);
    }
 }

diff --git a/src/common/blkreftable.c b/src/common/blkreftable.c
index bf0b563a38e23e03978dac41d522a0b78a2de604..bfa6f7ab5d8d7809b7806f55a1a6ddc10e798f46 100644 (file)
--- a/src/common/blkreftable.c
+++ b/src/common/blkreftable.c
@@ -988,8 +988,7 @@ BlockRefTableEntryMarkBlockModified(BlockRefTableEntry *entry,
         */
        max_chunks = Max(16, entry->nchunks);
        while (max_chunks < chunkno + 1)
-           chunkno *= 2;
-       Assert(max_chunks > chunkno);
+           max_chunks *= 2;
        extra_chunks = max_chunks - entry->nchunks;
 
        if (entry->nchunks == 0)