ctdb-server: Coverity fixes

Signed-off-by: Amitay Isaacs <amitay@gmail.com>
Reviewed-by: Michael Adam <obnox@samba.org>

diff --git a/ctdb/server/ctdb_banning.c b/ctdb/server/ctdb_banning.c
index e6df4b9bfc4357148dd824e678057b441ba5400d..13d97c84b13d4e9ca1e1419449eacf7d8a0c4f10 100644 (file)
--- a/ctdb/server/ctdb_banning.c
+++ b/ctdb/server/ctdb_banning.c
@@ -84,7 +84,7 @@ int32_t ctdb_control_set_ban_state(struct ctdb_context *ctdb, TDB_DATA indata)
        DEBUG(DEBUG_INFO,("SET BAN STATE\n"));
 
        if (bantime->pnn != ctdb->pnn) {
-               if (bantime->pnn < 0 || bantime->pnn >= ctdb->num_nodes) {
+               if (bantime->pnn >= ctdb->num_nodes) {
                        DEBUG(DEBUG_ERR,(__location__ " ERROR: Invalid ban request. PNN:%d is invalid. Max nodes %d\n", bantime->pnn, ctdb->num_nodes));
                        return -1;
                }

diff --git a/ctdb/server/ctdb_control.c b/ctdb/server/ctdb_control.c
index 99319ac523c4092f369ef8ffac5559ec88f31132..581c478d9dd520be78e2826c907cdbbad886d34a 100644 (file)
--- a/ctdb/server/ctdb_control.c
+++ b/ctdb/server/ctdb_control.c
@@ -50,6 +50,12 @@ int32_t ctdb_dump_memory(struct ctdb_context *ctdb, TDB_DATA *outdata)
        }
        talloc_report_full(NULL, f);
        fsize = ftell(f);
+       if (fsize == -1) {
+               DEBUG(DEBUG_ERR, (__location__ " Unable to get file size - %s\n",
+                                 strerror(errno)));
+               fclose(f);
+               return -1;
+       }
        rewind(f);
        outdata->dptr = talloc_size(outdata, fsize);
        if (outdata->dptr == NULL) {

diff --git a/ctdb/server/ctdb_daemon.c b/ctdb/server/ctdb_daemon.c
index cbe6b23ce0e8ca357a1d03e06cd29f926739385f..50b2de327ecfb00d8c29257215fbfac3e0e6f799 100644 (file)
--- a/ctdb/server/ctdb_daemon.c
+++ b/ctdb/server/ctdb_daemon.c
@@ -978,7 +978,7 @@ static int ux_socket_bind(struct ctdb_context *ctdb)
 
        memset(&addr, 0, sizeof(addr));
        addr.sun_family = AF_UNIX;
-       strncpy(addr.sun_path, ctdb->daemon.name, sizeof(addr.sun_path));
+       strncpy(addr.sun_path, ctdb->daemon.name, sizeof(addr.sun_path)-1);
 
        /* First check if an old ctdbd might be running */
        if (connect(ctdb->daemon.sd,

diff --git a/ctdb/server/ctdb_lock.c b/ctdb/server/ctdb_lock.c
index fc437b0fa7f8b950a32ee2f7a37499baa32e134f..bb66f9496320665ade63b5ee2f477b2823477a10 100644 (file)
--- a/ctdb/server/ctdb_lock.c
+++ b/ctdb/server/ctdb_lock.c
@@ -259,7 +259,7 @@ static int ctdb_lockall_unmark(struct ctdb_context *ctdb)
 {
        uint32_t priority;
 
-       for (priority=NUM_DB_PRIORITIES; priority>=0; priority--) {
+       for (priority=NUM_DB_PRIORITIES; priority>0; priority--) {
                if (ctdb_db_iterator(ctdb, priority, db_lock_unmark_handler, NULL) != 0) {
                        return -1;
                }
@@ -448,8 +448,11 @@ static void ctdb_lock_handler(struct tevent_context *ev,
        }
 
        /* Read the status from the child process */
-       read(lock_ctx->fd[0], &c, 1);
-       locked = (c == 0 ? true : false);
+       if (read(lock_ctx->fd[0], &c, 1) != 1) {
+               locked = false;
+       } else {
+               locked = (c == 0 ? true : false);
+       }
 
        /* Update statistics */
        CTDB_DECREMENT_STAT(lock_ctx->ctdb, locks.num_pending);
@@ -998,7 +1001,7 @@ struct lock_request *ctdb_lock_alldb_prio(struct ctdb_context *ctdb,
                                          void (*callback)(void *, bool),
                                          void *private_data)
 {
-       if (priority < 0 || priority > NUM_DB_PRIORITIES) {
+       if (priority < 1 || priority > NUM_DB_PRIORITIES) {
                DEBUG(DEBUG_ERR, ("Invalid db priority: %u\n", priority));
                return NULL;
        }

diff --git a/ctdb/server/ctdb_logging.c b/ctdb/server/ctdb_logging.c
index 218186e7f04c4b563cc34f854bb3e4d7b9d45f11..17c5c35ae7e68e9e094bcf76268d07731d9e83f5 100644 (file)
--- a/ctdb/server/ctdb_logging.c
+++ b/ctdb/server/ctdb_logging.c
@@ -38,7 +38,6 @@ struct ctdb_syslog_state {
 
 static int syslogd_is_started = 0;
 
-
 /* called when child is finished
  * this is for the syslog daemon, we can not use DEBUG here
  */
@@ -60,6 +59,10 @@ static void ctdb_syslog_handler(struct event_context *ev, struct fd_event *fde,
                return;
        }
        msg = (struct syslog_message *)str;
+       if (msg->len >= (sizeof(str) - offsetof(struct syslog_message, message))) {
+               msg->len = (sizeof(str)-1) - offsetof(struct syslog_message, message);
+       }
+       msg->message[msg->len] = '\0';
 
        syslog(msg->level, "%s", msg->message);
 }
@@ -401,8 +404,9 @@ static void write_to_log(struct ctdb_log_state *log,
                        do_debug("%*.*s\n", len, len, buf);
                }
                /* log it in the eventsystem as well */
-               if (log->logfn)
+               if (log && log->logfn) {
                        log->logfn(log->buf, len, log->logfn_private);
+               }
        }
 }
 

diff --git a/ctdb/server/ctdb_recover.c b/ctdb/server/ctdb_recover.c
index 1cbcc59abecb7a72949daf7442fd406d4a610b73..414f5b1f8d948b603cca38cc6f330162f45b56c9 100644 (file)
--- a/ctdb/server/ctdb_recover.c
+++ b/ctdb/server/ctdb_recover.c
@@ -1147,7 +1147,11 @@ static int store_tdb_record(struct ctdb_context *ctdb,
 
        data2 = tdb_fetch(ctdb_db->ltdb->tdb, key);
        if (data2.dptr == NULL || data2.dsize < sizeof(struct ctdb_ltdb_header)) {
-               tdb_store(ctdb_db->ltdb->tdb, key, data, 0);
+               if (tdb_store(ctdb_db->ltdb->tdb, key, data, 0) == -1) {
+                       DEBUG(DEBUG_ERR, (__location__ "Failed to store record\n"));
+                       ret = -1;
+                       goto done;
+               }
                DEBUG(DEBUG_INFO, (__location__ " Stored record\n"));
                ret = 0;
                goto done;

diff --git a/ctdb/server/ctdb_recoverd.c b/ctdb/server/ctdb_recoverd.c
index 5caf7c05eaa5129f4e56bab2b45012ed2a936a2c..8b4b6d0298bb73564db299da391abafdedf8ffc1 100644 (file)
--- a/ctdb/server/ctdb_recoverd.c
+++ b/ctdb/server/ctdb_recoverd.c
@@ -535,15 +535,17 @@ static int create_missing_remote_databases(struct ctdb_context *ctdb, struct ctd
                                continue;
                        }
                        /* ok so we need to create this database */
-                       ctdb_ctrl_getdbname(ctdb, CONTROL_TIMEOUT(), pnn, dbmap->dbs[db].dbid, 
-                                           mem_ctx, &name);
+                       ret = ctdb_ctrl_getdbname(ctdb, CONTROL_TIMEOUT(), pnn,
+                                                 dbmap->dbs[db].dbid, mem_ctx,
+                                                 &name);
                        if (ret != 0) {
                                DEBUG(DEBUG_ERR, (__location__ " Unable to get dbname from node %u\n", pnn));
                                return -1;
                        }
-                       ctdb_ctrl_createdb(ctdb, CONTROL_TIMEOUT(), nodemap->nodes[j].pnn, 
-                                          mem_ctx, name,
-                                          dbmap->dbs[db].flags & CTDB_DB_FLAGS_PERSISTENT);
+                       ret = ctdb_ctrl_createdb(ctdb, CONTROL_TIMEOUT(),
+                                                nodemap->nodes[j].pnn,
+                                                mem_ctx, name,
+                                                dbmap->dbs[db].flags & CTDB_DB_FLAGS_PERSISTENT);
                        if (ret != 0) {
                                DEBUG(DEBUG_ERR, (__location__ " Unable to create remote db:%s\n", name));
                                return -1;
@@ -2035,7 +2037,12 @@ static int do_recovery(struct ctdb_recoverd *rec,
 
        /* send a message to all clients telling them that the cluster 
           has been reconfigured */
-       ctdb_client_send_message(ctdb, CTDB_BROADCAST_CONNECTED, CTDB_SRVID_RECONFIGURE, tdb_null);
+       ret = ctdb_client_send_message(ctdb, CTDB_BROADCAST_CONNECTED,
+                                      CTDB_SRVID_RECONFIGURE, tdb_null);
+       if (ret != 0) {
+               DEBUG(DEBUG_ERR, (__location__ " Failed to send reconfigure message\n"));
+               return -1;
+       }
 
        DEBUG(DEBUG_NOTICE, (__location__ " Recovery complete\n"));
 
@@ -2206,9 +2213,7 @@ static int send_election_request(struct ctdb_recoverd *rec, uint32_t pnn)
 
        /* send an election message to all active nodes */
        DEBUG(DEBUG_INFO,(__location__ " Send election request to all active nodes\n"));
-       ctdb_client_send_message(ctdb, CTDB_BROADCAST_ALL, srvid, election_data);
-
-       return 0;
+       return ctdb_client_send_message(ctdb, CTDB_BROADCAST_ALL, srvid, election_data);
 }
 
 /*
@@ -2229,7 +2234,12 @@ static void unban_all_nodes(struct ctdb_context *ctdb)
        for (i=0;i<nodemap->num;i++) {
                if ( (!(nodemap->nodes[i].flags & NODE_FLAGS_DISCONNECTED))
                  && (nodemap->nodes[i].flags & NODE_FLAGS_BANNED) ) {
-                       ctdb_ctrl_modflags(ctdb, CONTROL_TIMEOUT(), nodemap->nodes[i].pnn, 0, NODE_FLAGS_BANNED);
+                       ret = ctdb_ctrl_modflags(ctdb, CONTROL_TIMEOUT(),
+                                                nodemap->nodes[i].pnn, 0,
+                                                NODE_FLAGS_BANNED);
+                       if (ret != 0) {
+                               DEBUG(DEBUG_ERR, (__location__ " failed to reset ban state\n"));
+                       }
                }
        }
 
@@ -2487,13 +2497,11 @@ static void disable_takeover_runs_handler(struct ctdb_context *ctdb,
                DEBUG(DEBUG_ERR,(__location__ " Wrong size for data :%lu "
                                 "expecting %lu\n", (long unsigned)data.dsize,
                                 (long unsigned)sizeof(struct srvid_request)));
-               ret = -EINVAL;
-               goto done;
+               return;
        }
        if (data.dptr == NULL) {
                DEBUG(DEBUG_ERR,(__location__ " No data received\n"));
-               ret = -EINVAL;
-               goto done;
+               return;
        }
 
        r = (struct srvid_request *)data.dptr;

diff --git a/ctdb/server/ctdb_takeover.c b/ctdb/server/ctdb_takeover.c
index 91f30302d777f2adebdd78ec5a14cab654e2a45f..04cdd6668d3a3fe7f19d5c250ad313e699606ed5 100644 (file)
--- a/ctdb/server/ctdb_takeover.c
+++ b/ctdb/server/ctdb_takeover.c
@@ -3386,7 +3386,7 @@ int32_t ctdb_control_get_public_ip_info(struct ctdb_context *ctdb,
                if (vnn->iface == cur) {
                        info->active_idx = i;
                }
-               strcpy(info->ifaces[i].name, cur->name);
+               strncpy(info->ifaces[i].name, cur->name, sizeof(info->ifaces[i].name)-1);
                info->ifaces[i].link_state = cur->link_up;
                info->ifaces[i].references = cur->references;
        }