Fix Coverity scan issues in open-vm-tools.

diff --git a/open-vm-tools/services/plugins/guestInfo/guestInfoServer.c b/open-vm-tools/services/plugins/guestInfo/guestInfoServer.c
index b283250483c4ce5ee594d5036e684f79378376a7..5c5ccc35f66184867cf48e240a66cf0a953fb7ff 100644 (file)
--- a/open-vm-tools/services/plugins/guestInfo/guestInfoServer.c
+++ b/open-vm-tools/services/plugins/guestInfo/guestInfoServer.c
@@ -1206,7 +1206,8 @@ GuestInfoSendNicInfo(ToolsAppCtx *ctx,             // IN
  * @param[in] infoSize  Size of disk info.
  *
  * @retval TRUE  Update sent successfully.
- * @retval FALSE Had trouble with serialization or transmission.
+ * @retval FALSE Had trouble with json string formatting, serialization or
+ *               transmission.
  *
  ******************************************************************************
  */
@@ -1222,7 +1223,7 @@ GuestInfoSendDiskInfoV1(ToolsAppCtx *ctx,             // IN
    char *infoReq;
    char *reply = NULL;
    size_t replyLen;
-   Bool status;
+   Bool status = FALSE;
 #ifdef _WIN32
    Bool reportUUID = VMTools_ConfigGetBoolean(ctx->config,
                                               CONFGROUPNAME_GUESTINFO,
@@ -1267,6 +1268,10 @@ GuestInfoSendDiskInfoV1(ToolsAppCtx *ctx,             // IN
 
    len = Str_Snprintf(tmpBuf, sizeof tmpBuf, headerFmt,
                       GUEST_DISK_INFO_COMMAND, DISK_INFO_VERSION_1);
+   if (len <= 0) {
+      goto exit;
+   }
+
    DynBuf_Append(&dynBuffer, tmpBuf, len);
    for (i = 0; i < pdi->numEntries; i++) {
       gchar *b64name;
@@ -1292,12 +1297,20 @@ GuestInfoSendDiskInfoV1(ToolsAppCtx *ctx,             // IN
                          b64name,
                          pdi->partitionList[i].freeBytes,
                          pdi->partitionList[i].totalBytes);
+      if (len <= 0) {
+         goto exit;
+      }
+
       DynBuf_Append(&dynBuffer, tmpBuf, len);
       g_free(b64name);
 
       if (pdi->partitionList[i].fsType[0] != '\0') {
          len = Str_Snprintf(tmpBuf, sizeof tmpBuf, jsonPerDiskFsTypeFmt,
                             pdi->partitionList[i].fsType);
+         if (len <= 0) {
+            goto exit;
+         }
+
          DynBuf_Append(&dynBuffer, tmpBuf, len);
       }
 #ifdef _WIN32
@@ -1305,6 +1318,10 @@ GuestInfoSendDiskInfoV1(ToolsAppCtx *ctx,             // IN
          if (pdi->partitionList[i].uuid[0] != '\0') {
             len = Str_Snprintf(tmpBuf, sizeof tmpBuf, jsonPerDiskUUIDFmt,
                                pdi->partitionList[i].uuid);
+            if (len <= 0) {
+               goto exit;
+            }
+
             DynBuf_Append(&dynBuffer, tmpBuf, len);
          }
       }
@@ -1316,11 +1333,19 @@ GuestInfoSendDiskInfoV1(ToolsAppCtx *ctx,             // IN
                        sizeof jsonPerDiskDevArrHdrFmt - 1);
          len = Str_Snprintf(tmpBuf, sizeof tmpBuf, jsonPerDiskDeviceFmt,
                             "", pdi->partitionList[i].diskDevNames[0]);
+         if (len <= 0) {
+            goto exit;
+         }
+
          DynBuf_Append(&dynBuffer, tmpBuf, len);
          for (idx = 1; idx < pdi->partitionList[i].diskDevCnt; idx++) {
             len = Str_Snprintf(tmpBuf, sizeof tmpBuf, jsonPerDiskDeviceFmt,
                                jsonPerDiskDeviceSep,
                                pdi->partitionList[i].diskDevNames[idx]);
+            if (len <= 0) {
+               goto exit;
+            }
+
             DynBuf_Append(&dynBuffer, tmpBuf, len);
          }
          DynBuf_Append(&dynBuffer, jsonPerDiskDevArrFmtFooter,
@@ -1352,9 +1377,10 @@ GuestInfoSendDiskInfoV1(ToolsAppCtx *ctx,             // IN
               __FUNCTION__, status, reply ? reply : "");
    }
 
-   DynBuf_Destroy(&dynBuffer);
    vm_free(reply);
 
+exit:
+   DynBuf_Destroy(&dynBuffer);
    return status;
 }
 

diff --git a/open-vm-tools/services/plugins/guestInfo/perfMonLinux.c b/open-vm-tools/services/plugins/guestInfo/perfMonLinux.c
index f353a262604c20869c3a175a6ba22baca1f38d3e..688f6492c509f8f2792ecd01d9c58c0ba547f7ae 100644 (file)
--- a/open-vm-tools/services/plugins/guestInfo/perfMonLinux.c
+++ b/open-vm-tools/services/plugins/guestInfo/perfMonLinux.c
@@ -352,6 +352,12 @@ GuestInfoStoreStatByID(GuestStatToolsID reportID,      // IN:
                     INT_AS_HASHKEY(reportID),
                     (void **) &stat);
 
+   /*
+    * Caller must not pass in a reportID that does not exist in the table.
+    */
+   ASSERT(stat != NULL);
+
+   /* coverity[var_deref_model] */
    GuestInfoStoreStat(stat, value);
 }
 
@@ -513,13 +519,15 @@ GuestInfoProcSimpleValue(GuestStatToolsID reportID,      // IN:
 
    HashTable_Lookup(collector->reportMap, INT_AS_HASHKEY(reportID),
                     (void **) &stat);
-   ASSERT(stat);
-   if (stat == NULL) {
-      g_warning("%s: Error stat ID %d not found.\n", __FUNCTION__, reportID);
-      return success;
-   }
 
+   /*
+    * Caller must not pass in a reportID that does not exist in the table.
+    */
+   ASSERT(stat != NULL);
+   ASSERT(stat->query != NULL);
    ASSERT(stat->query->sourceFile);
+
+   /* coverity[var_deref_op] */
    fp = Posix_Fopen(stat->query->sourceFile, "r");
    if (fp == NULL) {
       g_warning("%s: Error opening %s.\n",
@@ -589,14 +597,21 @@ GuestInfoDeriveSwapData(GuestInfoCollector *collector)  // IN/OUT:
                     INT_AS_HASHKEY(GuestStatID_SwapSpaceRemaining),
                     (void **) &swapSpaceRemaining);
 
+   ASSERT(swapFilesMax != NULL &&
+          swapFilesCurrent != NULL &&
+          swapSpaceUsed != NULL &&
+          swapSpaceRemaining != NULL);  // Must be in the table
+
    /*
     * Start by getting SwapTotal (from Id_SwapFilesCurrent).
     * Set Id_SwapFilesMax to that if it doesn't have its own opinion.
     */
-   if ((swapFilesCurrent != NULL) && (swapFilesCurrent->err == 0)) {
+   /* coverity[var_deref_op] */
+   if (swapFilesCurrent->err == 0) {
       swapTotal = swapFilesCurrent->value;
 
-      if ((swapFilesMax != NULL) && (swapFilesMax->err != 0)) {
+      /* coverity[var_deref_op] */
+      if (swapFilesMax->err != 0) {
          swapFilesMax->value = swapTotal;
          swapFilesMax->count = 1;
          swapFilesMax->err = 0;
@@ -607,12 +622,15 @@ GuestInfoDeriveSwapData(GuestInfoCollector *collector)  // IN/OUT:
        * Set Id_SwapSpaceUsed to SwapTotal-SwapFree if it doesn't have its
        * own opinion.
        */
-      if ((swapSpaceRemaining != NULL) && (swapSpaceRemaining->err == 0)) {
+      /* coverity[var_deref_op] */
+      if (swapSpaceRemaining->err == 0) {
          swapFree = swapSpaceRemaining->value;
 
          ASSERT(swapTotal >= swapFree);
          swapUsed = (swapTotal >= swapFree) ? swapTotal - swapFree : 0;
-         if ((swapSpaceUsed != NULL) && (swapSpaceUsed->err != 0)) {
+
+         /* coverity[var_deref_op] */
+         if (swapSpaceUsed->err != 0) {
             swapSpaceUsed->value = swapUsed;
             swapSpaceUsed->count = 1;
             swapSpaceUsed->err = 0;
@@ -649,13 +667,13 @@ GuestInfoDecreaseCpuRunQueueByOne(GuestInfoCollector *collector)  // IN/OUT:
                     INT_AS_HASHKEY(GuestStatID_Linux_CpuRunQueue),
                     (void **) &stat);
 
-   ASSERT(stat != NULL);
+   ASSERT(stat != NULL);  // Must be in the table
    ASSERT(stat->err == 0);
    ASSERT(stat->count == 1);
-   if (stat != NULL && stat->err == 0 && stat->count == 1) {
-      if (stat->value > 0) {
-         stat->value--;
-      }
+
+   /* coverity[var_deref_op] */
+   if (stat->value > 0) {
+      stat->value--;
    }
 }
 
@@ -818,7 +836,7 @@ GuestInfoCollect(GuestInfoCollector *collector)  // IN/OUT:
 
    /* Reset all values */
    for (i = 0; i < collector->numStats; i++) {
-      GuestInfoStat *stat = &collector->stats[i];
+      stat = &collector->stats[i];
 
       stat->err = ENOENT;  // There is no data here
       stat->count = 0;
@@ -853,8 +871,9 @@ GuestInfoCollect(GuestInfoCollector *collector)  // IN/OUT:
                     INT_AS_HASHKEY(GuestStatID_MemPhysUsable),
                     (void **) &stat);
 
-   ASSERT(stat != NULL);  // Must be in table
+   ASSERT(stat != NULL);  // Must be in the table
 
+   /* coverity[var_deref_op] */
    if (stat->err == 0) {
       stat->value *= (pageSize / 1024); // Convert pages to KiB
    } else {
@@ -864,7 +883,10 @@ GuestInfoCollect(GuestInfoCollector *collector)  // IN/OUT:
                        INT_AS_HASHKEY(GuestStatID_Linux_MemTotal),
                        (void **) &memTotal);
 
-      if ((memTotal != NULL) && (memTotal->err == 0)) {
+      ASSERT(memTotal != NULL);  // Must be in the table
+
+      /* coverity[var_deref_op] */
+      if (memTotal->err == 0) {
          stat->err = 0;
          stat->count = 1;
          stat->value = memTotal->value;
@@ -909,7 +931,10 @@ GuestInfoLegacy(GuestInfoCollector *current,  // IN: current collection
                     INT_AS_HASHKEY(GuestStatID_MemPhysUsable),
                     (void **) &stat);
 
-   if ((stat != NULL) && (stat->err == 0)) {
+   ASSERT(stat != NULL);  // Must be in the table
+
+   /* coverity[var_deref_op] */
+   if (stat->err == 0) {
       legacy->memTotal = stat->value;
       legacy->flags |= MEMINFO_MEMTOTAL;
    }
@@ -919,7 +944,10 @@ GuestInfoLegacy(GuestInfoCollector *current,  // IN: current collection
                     INT_AS_HASHKEY(GuestStatID_Linux_HugePagesTotal),
                     (void **) &stat);
 
-   if ((stat != NULL) && (stat->err == 0)) {
+   ASSERT(stat != NULL);  // Must be in the table
+
+   /* coverity[var_deref_op] */
+   if (stat->err == 0) {
       legacy->hugePagesTotal = stat->value;
       legacy->flags |= MEMINFO_HUGEPAGESTOTAL;
    }
@@ -1072,10 +1100,14 @@ GuestInfoAppendRate(Bool emitNameSpace,             // IN:
                     INT_AS_HASHKEY(reportID),
                     (void **) &previousStat);
 
+   ASSERT(currentStat != NULL &&
+          previousStat != NULL);  // Must be in the table
+
+   /* coverity[var_deref_op] */
    if (current->timeData &&
        previous->timeData &&
-       ((currentStat != NULL) && (currentStat->err == 0)) &&
-       ((previousStat != NULL) && (previousStat->err == 0))) {
+       currentStat->err == 0 &&
+       previousStat->err == 0) {
       double timeDelta = current->timeStamp - previous->timeStamp;
       double valueDelta;
 
@@ -1113,7 +1145,7 @@ GuestInfoAppendRate(Bool emitNameSpace,             // IN:
       errnoValue = 0;
    }
 
-   if (currentStat != NULL) {
+   {
       float valueFloat;
       void *valuePointer;
       size_t valueSize;
@@ -1165,17 +1197,14 @@ GuestInfoDeriveMemNeeded(GuestInfoCollector *collector)  // IN/OUT:
    GuestInfoStat *memAvail = NULL;
    GuestInfoStat *memPhysUsable = NULL;
 
-   HashTable_Lookup(collector->reportMap,
-                    INT_AS_HASHKEY(GuestStatID_MemPhysUsable),
-                    (void **) &memPhysUsable);
-
-   ASSERT(memPhysUsable != NULL);
-
    HashTable_Lookup(collector->reportMap,
                     INT_AS_HASHKEY(GuestStatID_Linux_MemAvailable),
                     (void **) &memAvail);
 
-   if ((memAvail != NULL) && (memAvail->err == 0)) {
+   ASSERT(memAvail != NULL);  // Must be in the table
+
+   /* coverity[var_deref_op] */
+   if (memAvail->err == 0) {
       memAvailable = memAvail->value;
    } else {
       GuestInfoStat *memFree = NULL;
@@ -1184,7 +1213,7 @@ GuestInfoDeriveMemNeeded(GuestInfoCollector *collector)  // IN/OUT:
       GuestInfoStat *memActiveFile = NULL;
       GuestInfoStat *memSlabReclaim = NULL;
       GuestInfoStat *memInactiveFile = NULL;
-      GuestInfoStat *lowWaterMark = NULL;
+      GuestInfoStat *memLowWaterMark = NULL;
 
       HashTable_Lookup(collector->reportMap,
                        INT_AS_HASHKEY(GuestStatID_MemFree),
@@ -1206,21 +1235,28 @@ GuestInfoDeriveMemNeeded(GuestInfoCollector *collector)  // IN/OUT:
                        (void **) &memInactiveFile);
       HashTable_Lookup(collector->reportMap,
                        INT_AS_HASHKEY(GuestStatID_Linux_LowWaterMark),
-                       (void **) &lowWaterMark);
-
-      if (((memFree != NULL) && (memFree->err == 0)) &&
-          ((memCache != NULL) && (memCache->err == 0)) &&
-          ((memBuffers != NULL) && (memBuffers->err == 0)) &&
-          (memActiveFile != NULL) &&
-          (memSlabReclaim != NULL) &&
-          (memInactiveFile != NULL) &&
-          ((lowWaterMark != NULL) && (lowWaterMark->err == 0))) {
+                       (void **) &memLowWaterMark);
+
+      ASSERT(memFree != NULL &&
+             memCache != NULL &&
+             memBuffers != NULL &&
+             memActiveFile != NULL &&
+             memSlabReclaim != NULL &&
+             memInactiveFile != NULL &&
+             memLowWaterMark != NULL);  // Must be in the table
+
+      /* coverity[var_deref_op] */
+      if (memFree->err == 0 &&
+          memCache->err == 0 &&
+          memBuffers->err == 0 &&
+          memLowWaterMark->err == 0) {
          uint64 pageCache;
          unsigned long kbPerPage = sysconf(_SC_PAGESIZE) / 1024UL;
-         uint64 lowWaterMarkValue = lowWaterMark->value * kbPerPage;
+         uint64 lowWaterMarkValue = memLowWaterMark->value * kbPerPage;
 
          memAvailable = memFree->value - lowWaterMarkValue;
 
+         /* coverity[var_deref_op] */
          if ((memActiveFile->err == 0) && (memInactiveFile->err == 0)) {
             pageCache = memActiveFile->value + memInactiveFile->value;
          } else {
@@ -1234,6 +1270,7 @@ GuestInfoDeriveMemNeeded(GuestInfoCollector *collector)  // IN/OUT:
          pageCache -= MIN(pageCache / 2, lowWaterMarkValue);
          memAvailable += pageCache;
 
+         /* coverity[var_deref_op] */
          if (memSlabReclaim->err == 0) {
             memAvailable += memSlabReclaim->value -
                             MIN(memSlabReclaim->value / 2, lowWaterMarkValue);
@@ -1249,6 +1286,13 @@ GuestInfoDeriveMemNeeded(GuestInfoCollector *collector)  // IN/OUT:
       }
    }
 
+   HashTable_Lookup(collector->reportMap,
+                    INT_AS_HASHKEY(GuestStatID_MemPhysUsable),
+                    (void **) &memPhysUsable);
+
+   ASSERT(memPhysUsable != NULL);  // Must be in the table
+
+   /* coverity[var_deref_op] */
    if (memPhysUsable->err == 0) {
       /*
        * Reserve 5% of physical RAM for surges.