Fix assorted infelicities in new SetWALSegSize() function.
authorTom Lane <[email protected]>
Sun, 24 Sep 2017 16:05:06 +0000 (12:05 -0400)
committerTom Lane <[email protected]>
Sun, 24 Sep 2017 16:05:06 +0000 (12:05 -0400)
* Failure to check for malloc failure (ok, pretty unlikely here, but
that's not an excuse).

* Leakage of open fd on read error, and of malloc'd buffer always.

* Incorrect assumption that a short read would set errno to zero.

* Failure to adhere to message style conventions (in particular,
not reporting errno where relevant; using "couldn't open" rather than
"could not open" is not really in line with project style either).

* Missing newlines on some messages.

Coverity spotted the leak problems; I noticed the rest while
fixing the leaks.

contrib/pg_standby/pg_standby.c

index 6aeca6e8f72acd35e763cc8a9001cc1848e01dcf..cb785971a98f346ee30326d3ce1d760636453d7f 100644 (file)
@@ -408,16 +408,21 @@ SetWALSegSize(void)
 {
    bool        ret_val = false;
    int         fd;
-   char       *buf = (char *) malloc(XLOG_BLCKSZ);
+
+   /* malloc this buffer to ensure sufficient alignment: */
+   char       *buf = (char *) pg_malloc(XLOG_BLCKSZ);
 
    Assert(WalSegSz == -1);
 
    if ((fd = open(WALFilePath, O_RDWR, 0)) < 0)
    {
-       fprintf(stderr, "%s: couldn't open WAL file \"%s\"\n",
-               progname, WALFilePath);
+       fprintf(stderr, "%s: could not open WAL file \"%s\": %s\n",
+               progname, WALFilePath, strerror(errno));
+       pg_free(buf);
        return false;
    }
+
+   errno = 0;
    if (read(fd, buf, XLOG_BLCKSZ) == XLOG_BLCKSZ)
    {
        XLogLongPageHeader longhdr = (XLogLongPageHeader) buf;
@@ -433,7 +438,6 @@ SetWALSegSize(void)
            fprintf(stderr,
                    "%s: WAL segment size must be a power of two between 1MB and 1GB, but the WAL file header specifies %d bytes\n",
                    progname, WalSegSz);
-       close(fd);
    }
    else
    {
@@ -444,17 +448,21 @@ SetWALSegSize(void)
        if (errno != 0)
        {
            if (debug)
-               fprintf(stderr, "could not read file \"%s\": %s",
+               fprintf(stderr, "could not read file \"%s\": %s\n",
                        WALFilePath, strerror(errno));
        }
        else
        {
            if (debug)
-               fprintf(stderr, "not enough data in file \"%s\"", WALFilePath);
+               fprintf(stderr, "not enough data in file \"%s\"\n",
+                       WALFilePath);
        }
    }
 
    fflush(stderr);
+
+   close(fd);
+   pg_free(buf);
    return ret_val;
 }