From: Victor Julien <victor@inliniac.net>
Date: Wed, 29 Feb 2012 13:32:32 +0000 (+0100)
Subject: Various improvements to error handling found by Coverity.
X-Git-Tag: suricata-1.3beta1~143
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=11bdf4838ff98381a5c7b80cd4eb412294657bee;p=thirdparty%2Fsuricata.git

Various improvements to error handling found by Coverity.
---

diff --git a/src/log-httplog.c b/src/log-httplog.c
index 1f5fc56daa..145e3591f0 100644
--- a/src/log-httplog.c
+++ b/src/log-httplog.c
@@ -403,12 +403,9 @@ void LogHttpLogExitPrintStats(ThreadVars *tv, void *data) {
  * */
 OutputCtx *LogHttpLogInitCtx(ConfNode *conf)
 {
-    LogFileCtx* file_ctx=LogFileNewCtx();
-
-    if(file_ctx == NULL)
-    {
-        SCLogError(SC_ERR_HTTP_LOG_GENERIC, "LogHttpLogInitCtx: Couldn't "
-                   "create new file_ctx");
+    LogFileCtx* file_ctx = LogFileNewCtx();
+    if(file_ctx == NULL) {
+        SCLogError(SC_ERR_HTTP_LOG_GENERIC, "couldn't create new file_ctx");
         return NULL;
     }
 
@@ -417,10 +414,15 @@ OutputCtx *LogHttpLogInitCtx(ConfNode *conf)
         return NULL;
     }
 
-    LogHttpFileCtx *httplog_ctx = SCCalloc(1, sizeof(LogHttpFileCtx));
-    if (httplog_ctx == NULL)
+    LogHttpFileCtx *httplog_ctx = SCMalloc(sizeof(LogHttpFileCtx));
+    if (httplog_ctx == NULL) {
+        LogFileFreeCtx(file_ctx);
         return NULL;
+    }
+    memset(httplog_ctx, 0x00, sizeof(LogHttpFileCtx));
+
     httplog_ctx->file_ctx = file_ctx;
+
     const char *extended = ConfNodeLookupChildValue(conf, "extended");
     if (extended == NULL) {
         httplog_ctx->flags |= LOG_HTTP_DEFAULT;
@@ -431,8 +433,12 @@ OutputCtx *LogHttpLogInitCtx(ConfNode *conf)
     }
 
     OutputCtx *output_ctx = SCCalloc(1, sizeof(OutputCtx));
-    if (output_ctx == NULL)
+    if (output_ctx == NULL) {
+        LogFileFreeCtx(file_ctx);
+        SCFree(httplog_ctx);
         return NULL;
+    }
+
     output_ctx->data = httplog_ctx;
     output_ctx->DeInit = LogHttpLogDeInitCtx;
 
diff --git a/src/log-pcap.c b/src/log-pcap.c
index 78e1b1afd5..0ec9bb93cc 100644
--- a/src/log-pcap.c
+++ b/src/log-pcap.c
@@ -171,6 +171,18 @@ int PcapLogCloseFile(ThreadVars *t, PcapLogData *pl) {
     return 0;
 }
 
+static void PcapFileNameFree(PcapFileName *pf) {
+    if (pf != NULL) {
+        if (pf->filename != NULL) {
+            SCFree(pf->filename);
+        }
+        if (pf->dirname != NULL) {
+            SCFree(pf->dirname);
+        }
+        SCFree(pf);
+    }
+}
+
 /**
  *  \brief Function to rotate pcaplog file
  *
@@ -199,9 +211,8 @@ int PcapLogRotateFile(ThreadVars *t, PcapLogData *pl) {
                  "failed to remove log file %s: %s",
                  pf->filename, strerror( errno ));
              TAILQ_REMOVE(&pcap_file_list, pf, next);
-             if (pf != NULL)
-                 free(pf);
 
+             PcapFileNameFree(pf);
              return -1;
          }
          else {
@@ -226,16 +237,15 @@ int PcapLogRotateFile(ThreadVars *t, PcapLogData *pl) {
                               "failed to remove sguil log %s: %s",
                               pf->dirname, strerror( errno ));
                           TAILQ_REMOVE(&pcap_file_list, pf, next);
-                          if (pf != NULL)
-                              free(pf);
 
+                          PcapFileNameFree(pf);
                           return -1;
                       }
                  }
              }
+
              TAILQ_REMOVE(&pcap_file_list, pf, next);
-             if (pf != NULL)
-                 free(pf);
+             PcapFileNameFree(pf);
 
              pl->file_cnt--;
          }
@@ -619,7 +629,6 @@ int PcapLogOpenFileCtx(PcapLogData *pl)
     memset(&ts, 0x00, sizeof(struct timeval));
     TimeGet(&ts);
 
-
     /* Place to store the name of our PCAP file */
     PcapFileName *pf = SCMalloc(sizeof(PcapFileName));
     if (pf == NULL) {
@@ -646,12 +655,7 @@ int PcapLogOpenFileCtx(PcapLogData *pl)
 #endif
         if ((pf->dirname = SCStrdup(dirfull)) == NULL) {
             SCLogError(SC_ERR_MEM_ALLOC, "Error allocating memory for directory name");
-            return -1;
-        }
-
-        if (strlen(pf->dirname) == 0) {
-            SCFree(pf->dirname);
-            return -1;
+            goto error;
         }
 
         if (pl->timestamp_format == TS_FORMAT_SEC) {
@@ -671,10 +675,14 @@ int PcapLogOpenFileCtx(PcapLogData *pl)
 
     if ((pf->filename = SCStrdup(pl->filename)) == NULL) {
         SCLogError(SC_ERR_MEM_ALLOC, "Error allocating memory. For filename");
-        return -1;
+        goto error;
     }
-    SCLogDebug("Opening pcap file log %s\n", pf->filename);
+    SCLogDebug("Opening pcap file log %s", pf->filename);
     TAILQ_INSERT_TAIL(&pcap_file_list, pf, next);
 
     return 0;
+
+error:
+    PcapFileNameFree(pf);
+    return -1;
 }
diff --git a/src/runmode-af-packet.c b/src/runmode-af-packet.c
index 28254093ed..1eb9c2e925 100644
--- a/src/runmode-af-packet.c
+++ b/src/runmode-af-packet.c
@@ -110,13 +110,15 @@ void *ParseAFPConfig(const char *iface)
     intmax_t value;
     int boolval;
 
-    if (iface == NULL) {
+    if (aconf == NULL) {
         return NULL;
     }
 
-    if (aconf == NULL) {
+    if (iface == NULL) {
+        SCFree(aconf);
         return NULL;
     }
+
     strlcpy(aconf->iface, iface, sizeof(aconf->iface));
     aconf->threads = 1;
     SC_ATOMIC_INIT(aconf->ref);
@@ -190,6 +192,7 @@ void *ParseAFPConfig(const char *iface)
         aconf->cluster_type = PACKET_FANOUT_CPU;
     } else {
         SCLogError(SC_ERR_INVALID_CLUSTER_TYPE,"invalid cluster-type %s",tmpctype);
+        SCFree(aconf);
         return NULL;
     }
 
diff --git a/src/runmode-pcap.c b/src/runmode-pcap.c
index 96a4893a31..c723279dd2 100644
--- a/src/runmode-pcap.c
+++ b/src/runmode-pcap.c
@@ -85,13 +85,15 @@ void *ParsePcapConfig(const char *iface)
     char *tmpctype;
     intmax_t value;
 
-    if (iface == NULL) {
+    if (aconf == NULL) {
         return NULL;
     }
 
-    if (aconf == NULL) {
+    if (iface == NULL) {
+        SCFree(aconf);
         return NULL;
     }
+
     strlcpy(aconf->iface, iface, sizeof(aconf->iface));
 
     aconf->buffer_size = 0;
diff --git a/src/runmode-pfring.c b/src/runmode-pfring.c
index bbc7eaaeb6..567b539834 100644
--- a/src/runmode-pfring.c
+++ b/src/runmode-pfring.c
@@ -114,13 +114,15 @@ void *OldParsePfringConfig(const char *iface)
     cluster_type default_ctype = CLUSTER_ROUND_ROBIN;
 #endif
 
-    if (iface == NULL) {
+    if (pfconf == NULL) {
         return NULL;
     }
 
-    if (pfconf == NULL) {
+    if (iface == NULL) {
+        SCFree(pfconf);
         return NULL;
     }
+
     strlcpy(pfconf->iface, iface, sizeof(pfconf->iface));
     pfconf->threads = 1;
     pfconf->cluster_id = 1;
@@ -167,6 +169,7 @@ void *OldParsePfringConfig(const char *iface)
         pfconf->ctype = (cluster_type)tmpctype;
     } else {
         SCLogError(SC_ERR_INVALID_CLUSTER_TYPE,"invalid cluster-type %s",tmpctype);
+        SCFree(pfconf);
         return NULL;
     }
 #endif
@@ -203,13 +206,15 @@ void *ParsePfringConfig(const char *iface)
     char *bpf_filter = NULL;
 #endif /* HAVE_PFRING_SET_BPF_FILTER */
 
-    if (iface == NULL) {
+    if (pfconf == NULL) {
         return NULL;
     }
 
-    if (pfconf == NULL) {
+    if (iface == NULL) {
+        SCFree(pfconf);
         return NULL;
     }
+
     memset(pfconf, 0, sizeof(PfringIfaceConfig));
     strlcpy(pfconf->iface, iface, sizeof(pfconf->iface));
     pfconf->threads = 1;
@@ -312,6 +317,7 @@ void *ParsePfringConfig(const char *iface)
             SCLogError(SC_ERR_INVALID_CLUSTER_TYPE,
                        "invalid cluster-type %s",
                        tmpctype);
+            SCFree(pfconf);
             return NULL;
         }
     }
diff --git a/src/source-af-packet.c b/src/source-af-packet.c
index 9378fe0a83..e42b0b2324 100644
--- a/src/source-af-packet.c
+++ b/src/source-af-packet.c
@@ -878,6 +878,7 @@ TmEcode ReceiveAFPThreadInit(ThreadVars *tv, void *initdata, void **data) {
     ptv->livedev = LiveGetDevice(ptv->iface);
     if (ptv->livedev == NULL) {
         SCLogError(SC_ERR_INVALID_VALUE, "Unable to find Live device");
+        SCFree(ptv);
         SCReturnInt(TM_ECODE_FAILED);
     }
 
diff --git a/src/source-pcap.c b/src/source-pcap.c
index b5b2ed27a5..4d3f59a876 100644
--- a/src/source-pcap.c
+++ b/src/source-pcap.c
@@ -341,6 +341,7 @@ TmEcode ReceivePcapThreadInit(ThreadVars *tv, void *initdata, void **data) {
     ptv->livedev = LiveGetDevice(pcapconfig->iface);
     if (ptv->livedev == NULL) {
         SCLogError(SC_ERR_INVALID_VALUE, "Unable to find Live device");
+        SCFree(ptv);
         SCReturnInt(TM_ECODE_FAILED);
     }
 
diff --git a/src/tm-threads.c b/src/tm-threads.c
index 75cf12d05f..bf1bb6912d 100644
--- a/src/tm-threads.c
+++ b/src/tm-threads.c
@@ -1216,7 +1216,10 @@ ThreadVars *TmThreadCreate(char *name, char *inq_name, char *inqh_name,
     return tv;
 
 error:
-    printf("ERROR: failed to setup a thread.\n");
+    SCLogError(SC_ERR_THREAD_CREATE, "failed to setup a thread");
+
+    if (tv != NULL)
+        SCFree(tv);
     return NULL;
 }
 
diff --git a/src/util-mem.h b/src/util-mem.h
index ac9c1ba2aa..6aedaa1fcf 100644
--- a/src/util-mem.h
+++ b/src/util-mem.h
@@ -157,9 +157,9 @@ SC_ATOMIC_EXTERN(unsigned int, engine_stage);
     ptrmem = malloc((a)); \
     if (ptrmem == NULL) { \
         if (SC_ATOMIC_GET(engine_stage) == SURICATA_INIT) {\
-            uintmax_t size = (uintmax_t)(a); \
+            uintmax_t scmalloc_size_ = (uintmax_t)(a); \
             SCLogError(SC_ERR_MEM_ALLOC, "SCMalloc failed: %s, while trying " \
-                "to allocate %"PRIuMAX" bytes", strerror(errno), size); \
+                "to allocate %"PRIuMAX" bytes", strerror(errno), scmalloc_size_); \
             SCLogError(SC_ERR_FATAL, "Out of memory. The engine cannot be initialized. Exiting..."); \
             exit(EXIT_FAILURE); \
         } \