Another round of Coverity fixes

Additional non-security issues/improvements spotted by Coverity.

In backend/libpq, no sense trying to protect against port->hba being
NULL after we've already dereferenced it in the switch() statement.

Prevent against possible overflow due to 32bit arithmitic in
basebackup throttling (not yet released, so no security concern).

Remove nonsensical check of array pointer against NULL in procarray.c,
looks to be a holdover from 9.1 and earlier when there were pointers
being used but now it's just an array.

Remove pointer check-against-NULL in tsearch/spell.c as we had already
dereferenced it above (in the strcmp()).

Remove dead code from adt/orderedsetaggs.c, isnull is checked
immediately after each tuplesort_getdatum() call and if true we return,
so no point checking it again down at the bottom.

Remove recently added minor error-condition memory leak in pg_regress.

diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 9d0c3893c8d565a7ed06f8aea31b6ad0ced95337..f03aa7edc22447571e0408f9d4f1a60de693668a 100644 (file)
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -214,6 +214,7 @@ static void
 auth_failed(Port *port, int status, char *logdetail)
 {
    const char *errstr;
+   char       *cdetail;
    int         errcode_return = ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION;
 
    /*
@@ -273,17 +274,12 @@ auth_failed(Port *port, int status, char *logdetail)
            break;
    }
 
-   if (port->hba)
-   {
-       char       *cdetail;
-
-       cdetail = psprintf(_("Connection matched pg_hba.conf line %d: \"%s\""),
-                          port->hba->linenumber, port->hba->rawline);
-       if (logdetail)
-           logdetail = psprintf("%s\n%s", logdetail, cdetail);
-       else
-           logdetail = cdetail;
-   }
+   cdetail = psprintf(_("Connection matched pg_hba.conf line %d: \"%s\""),
+                      port->hba->linenumber, port->hba->rawline);
+   if (logdetail)
+       logdetail = psprintf("%s\n%s", logdetail, cdetail);
+   else
+       logdetail = cdetail;
 
    ereport(FATAL,
            (errcode(errcode_return),

diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index d68a1533602bb5620fd6c58ac8e8a25556a29d1f..f611f591b61180d114982d5106896012a8589838 100644 (file)
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -227,7 +227,8 @@ perform_base_backup(basebackup_options *opt, DIR *tblspcdir)
        /* Setup and activate network throttling, if client requested it */
        if (opt->maxrate > 0)
        {
-           throttling_sample = opt->maxrate * 1024 / THROTTLING_FREQUENCY;
+           throttling_sample =
+               (int64) opt->maxrate * (int64) 1024 / THROTTLING_FREQUENCY;
 
            /*
             * The minimum amount of time for throttling_sample

diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index 082115b4fffdf2edb363d7eee20daa030780b02a..eac418442d3f1fa5b991958ecbd2e808f1b3286a 100644 (file)
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -2302,9 +2302,9 @@ MinimumActiveBackends(int min)
        volatile PGXACT *pgxact = &allPgXact[pgprocno];
 
        /*
-        * Since we're not holding a lock, need to check that the pointer is
-        * valid. Someone holding the lock could have incremented numProcs
-        * already, but not yet inserted a valid pointer to the array.
+        * Since we're not holding a lock, need to be prepared to deal with
+        * garbage, as someone could have incremented numPucs but not yet
+        * filled the structure.
         *
         * If someone just decremented numProcs, 'proc' could also point to a
         * PGPROC entry that's no longer in the array. It still points to a
@@ -2312,9 +2312,6 @@ MinimumActiveBackends(int min)
         * free list and are recycled. Its contents are nonsense in that case,
         * but that's acceptable for this function.
         */
-       if (proc == NULL)
-           continue;
-
        if (proc == MyProc)
            continue;           /* do not count myself */
        if (pgxact->xid == InvalidTransactionId)

diff --git a/src/backend/tsearch/spell.c b/src/backend/tsearch/spell.c
index 1bc226d3347c939bb58c72eb74dc0e61b257ec15..530c6eddb8cacf09fc9a57ab9dea698201cf647f 100644 (file)
--- a/src/backend/tsearch/spell.c
+++ b/src/backend/tsearch/spell.c
@@ -404,7 +404,7 @@ NIAddAffix(IspellDict *Conf, int flag, char flagflags, const char *mask, const c
        Affix->issimple = 0;
        Affix->isregis = 1;
        RS_compile(&(Affix->reg.regis), (type == FF_SUFFIX) ? true : false,
-                  (mask && *mask) ? mask : VoidString);
+                  *mask ? mask : VoidString);
    }
    else
    {

diff --git a/src/backend/utils/adt/orderedsetaggs.c b/src/backend/utils/adt/orderedsetaggs.c
index e5324e28699d41bef23673e77c78f0503681325e..99577a549e64e2675d494dee99d38d5dfe612b4d 100644 (file)
--- a/src/backend/utils/adt/orderedsetaggs.c
+++ b/src/backend/utils/adt/orderedsetaggs.c
@@ -585,10 +585,7 @@ percentile_cont_final_common(FunctionCallInfo fcinfo,
     * trouble, since the cleanup callback will clear the tuplesort later.
     */
 
-   if (isnull)
-       PG_RETURN_NULL();
-   else
-       PG_RETURN_DATUM(val);
+   PG_RETURN_DATUM(val);
 }
 
 /*

diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 541ce8b33d393b4c4476a90830982db32a74d326..438b95fe59aeb742007171edfeb81e3365d31b32 100644 (file)
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -1154,11 +1154,17 @@ get_alternative_expectfile(const char *expectfile, int i)
 {
    char       *last_dot;
    int         ssize = strlen(expectfile) + 2 + 1;
-   char       *tmp = (char *) malloc(ssize);
-   char       *s = (char *) malloc(ssize);
+   char       *tmp;
+   char       *s;
 
-   if (!tmp || !s)
+   if (!(tmp = (char*) malloc(ssize)))
        return NULL;
+   
+   if (!(s = (char*) malloc(ssize)))
+   {
+       free(tmp);
+       return NULL;
+   }
 
    strcpy(tmp, expectfile);
    last_dot = strrchr(tmp, '.');