Harden some scope code.

Signed-off-by: Samuel K. Gutierrez <[email protected]>
hpc · Jul 24, 2024 · d2967ad · d2967ad
1 parent d02ef1f
commit d2967ad
Show file tree

Hide file tree

Showing 4 changed files with 72 additions and 81 deletions.
diff --git a/src/qvi-bbuff-rmi.h b/src/qvi-bbuff-rmi.h
@@ -30,9 +30,9 @@
 #define QVI_BBUFF_RMI_H
 
 #include "qvi-common.h"
+#include "qvi-bbuff.h"
 #include "qvi-hwloc.h"
 #include "qvi-hwpool.h"
-#include "qvi-utils.h"
 
 // 'Null' cpuset representation as a string.
 static constexpr cstr_t QV_BUFF_RMI_NULL_CPUSET = "";
@@ -553,30 +553,7 @@ qvi_bbuff_rmi_pack_item_impl(
     qvi_bbuff_t *buff,
     const qvi_hwpool_s *data
 ) {
-    // Pack CPU.
-    int rc = qvi_bbuff_rmi_pack_item(buff, data->m_cpu);
-    if (rc != QV_SUCCESS) return rc;
-    // Pack ndevinfos
-    const size_t ndev = data->m_devs.size();
-    rc = qvi_bbuff_rmi_pack_item(buff, ndev);
-    if (rc != QV_SUCCESS) return rc;
-    // Pack devices.
-    for (const auto &dev : data->m_devs) {
-        rc = qvi_bbuff_rmi_pack_item(buff, dev.second.get());
-        if (rc != QV_SUCCESS) return rc;
-    }
-    return rc;
-}
-
-/**
- * Packs const qvi_hwpool_s *
- */
-inline int
-qvi_bbuff_rmi_pack_item(
-    qvi_bbuff_t *buff,
-    const qvi_hwpool_s *data
-) {
-    return qvi_bbuff_rmi_pack_item_impl(buff, data);
+    return data->pack(buff);
 }
 
 /**
@@ -953,47 +930,7 @@ qvi_bbuff_rmi_unpack_item(
     byte_t *buffpos,
     size_t *bytes_written
 ) {
-    qvi_hwpool_s *ihwp = nullptr;
-    int rc = qvi_new(&ihwp);
-    if (rc != QV_SUCCESS) return rc;
-
-    size_t bw = 0, total_bw = 0;
-
-    // Unpack CPU.
-    rc = qvi_bbuff_rmi_unpack_item(
-        ihwp->m_cpu, buffpos, &bw
-    );
-    if (rc != QV_SUCCESS) return rc;
-    total_bw += bw;
-    buffpos += bw;
-    // Unpack number of devices.
-    size_t ndev = 0;
-    rc = qvi_bbuff_rmi_unpack_item(
-        &ndev, buffpos, &bw
-    );
-    if (rc != QV_SUCCESS) goto out;
-    total_bw += bw;
-    buffpos += bw;
-    // Unpack devices.
-    for (size_t i = 0; i < ndev; ++i) {
-        qvi_hwpool_dev_s dev;
-        rc = qvi_bbuff_rmi_unpack_item(
-            &dev, buffpos, &bw
-        );
-        total_bw += bw;
-        buffpos += bw;
-        if (rc != QV_SUCCESS) break;
-
-        rc = ihwp->add_device(dev);
-        if (rc != QV_SUCCESS) break;
-    }
-out:
-    if (rc != QV_SUCCESS) {
-        total_bw = 0;
-    }
-    *bytes_written = total_bw;
-    *hwp = ihwp;
-    return rc;
+    return qvi_hwpool_s::unpack(buffpos, bytes_written, hwp);
 }
 
 /**

diff --git a/src/qvi-hwpool.cc b/src/qvi-hwpool.cc
@@ -43,7 +43,6 @@
 // approach using the device IDs instead of the bit positions.
 
 #include "qvi-hwpool.h"
-#include "qvi-bbuff.h"
 #include "qvi-bbuff-rmi.h"
 #include "qvi-utils.h"
 
@@ -187,13 +186,13 @@ qvi_hwpool_s::initialize(
 }
 
 const qvi_hwloc_bitmap_s &
-qvi_hwpool_s::cpuset(void)
+qvi_hwpool_s::cpuset(void) const
 {
     return m_cpu.cpuset;
 }
 
 const qvi_hwpool_devs_t &
-qvi_hwpool_s::devices(void)
+qvi_hwpool_s::devices(void) const
 {
     return m_devs;
 }
@@ -232,16 +231,69 @@ qvi_hwpool_s::release_devices(void)
 int
 qvi_hwpool_s::pack(
     qvi_bbuff_t *buff
-) {
-    return qvi_bbuff_rmi_pack(buff, this);
+) const {
+    // Pack the CPU.
+    int rc = qvi_bbuff_rmi_pack_item(buff, m_cpu);
+    if (qvi_unlikely(rc != QV_SUCCESS)) return rc;
+    // Pack the number of devices.
+    const size_t ndev = m_devs.size();
+    rc = qvi_bbuff_rmi_pack_item(buff, ndev);
+    if (qvi_unlikely(rc != QV_SUCCESS)) return rc;
+    // Pack the devices.
+    for (const auto &dev : m_devs) {
+        rc = qvi_bbuff_rmi_pack_item(buff, dev.second.get());
+        if (qvi_unlikely(rc != QV_SUCCESS)) return rc;
+    }
+    return rc;
 }
 
 int
 qvi_hwpool_s::unpack(
-    qvi_bbuff_t *buff,
+    byte_t *buffpos,
+    size_t *bytes_written,
     qvi_hwpool_s **hwp
 ) {
-    return qvi_bbuff_rmi_unpack(qvi_bbuff_data(buff), hwp);
+    size_t bw = 0, total_bw = 0;
+    // Create the new hardware pool.
+    qvi_hwpool_s *ihwp = nullptr;
+    int rc = qvi_new(&ihwp);
+    if (qvi_unlikely(rc != QV_SUCCESS)) goto out;
+    // Unpack the CPU into the hardare pool.
+    rc = qvi_bbuff_rmi_unpack_item(
+        ihwp->m_cpu, buffpos, &bw
+    );
+    if (qvi_unlikely(rc != QV_SUCCESS)) goto out;
+    total_bw += bw;
+    buffpos += bw;
+    // Unpack the number of devices.
+    size_t ndev;
+    rc = qvi_bbuff_rmi_unpack_item(
+        &ndev, buffpos, &bw
+    );
+    if (qvi_unlikely(rc != QV_SUCCESS)) goto out;
+    total_bw += bw;
+    buffpos += bw;
+    // Unpack and add the devices.
+    for (size_t i = 0; i < ndev; ++i) {
+        qvi_hwpool_dev_s dev;
+        rc = qvi_bbuff_rmi_unpack_item(
+            &dev, buffpos, &bw
+        );
+        if (qvi_unlikely(rc != QV_SUCCESS)) break;
+        total_bw += bw;
+        buffpos += bw;
+        //
+        rc = ihwp->add_device(dev);
+        if (qvi_unlikely(rc != QV_SUCCESS)) break;
+    }
+out:
+    if (qvi_unlikely(rc != QV_SUCCESS)) {
+        total_bw = 0;
+        qvi_delete(&ihwp);
+    }
+    *bytes_written = total_bw;
+    *hwp = ihwp;
+    return rc;
 }
 
 /**

diff --git a/src/qvi-hwpool.h b/src/qvi-hwpool.h
@@ -84,11 +84,11 @@ using qvi_hwpool_devs_t = std::multimap<
 >;
 
 struct qvi_hwpool_s {
+private:
     /** The hardware pool's CPU. */
     qvi_hwpool_cpu_s m_cpu;
     /** The hardware pool's devices. */
     qvi_hwpool_devs_t m_devs;
-private:
     /**
      * Adds all devices with affinity to the
      * provided, initialized hardware resource pool.
@@ -121,12 +121,12 @@ struct qvi_hwpool_s {
      * Returns a const reference to the hardware pool's cpuset.
      */
     const qvi_hwloc_bitmap_s &
-    cpuset(void);
+    cpuset(void) const;
     /**
      * Returns a const reference to the hardware pool's devices.
      */
     const qvi_hwpool_devs_t &
-    devices(void);
+    devices(void) const;
     /**
      * Returns the number of objects in the hardware pool.
      */
@@ -154,13 +154,14 @@ struct qvi_hwpool_s {
     int
     pack(
         qvi_bbuff_t *buff
-    );
+    ) const;
     /**
      * Unpacks the buffer and creates a new hardware pool instance.
      */
     static int
     unpack(
-        qvi_bbuff_t *buff,
+        byte_t *buffpos,
+        size_t *bytes_written,
         qvi_hwpool_s **hwp
     );
 };

diff --git a/src/qvi-scope.cc b/src/qvi-scope.cc
@@ -21,6 +21,7 @@
 #include "qvi-bbuff.h"
 #include "qvi-task.h"
 #include "qvi-rmi.h"
+#include "qvi-bbuff-rmi.h"
 #include "qvi-hwpool.h"
 #include "qvi-map.h"
 #include "qvi-utils.h"
@@ -475,8 +476,8 @@ gather_hwpools(
         rxpools.resize(group_size);
         // Unpack the hwpools.
         for (uint_t i = 0; i < group_size; ++i) {
-            rc = qvi_hwpool_s::unpack(
-                bbuffs[i], &rxpools[i]
+            rc = qvi_bbuff_rmi_unpack(
+                qvi_bbuff_data(bbuffs[i]), &rxpools[i]
             );
             if (rc != QV_SUCCESS) break;
         }
@@ -572,7 +573,7 @@ scatter_hwpools(
     rc = group->scatter(txbuffs.data(), root, &rxbuff);
     if (rc != QV_SUCCESS) goto out;
 
-    rc = qvi_hwpool_s::unpack(rxbuff, pool);
+    rc = qvi_bbuff_rmi_unpack(qvi_bbuff_data(rxbuff), pool);
 out:
     for (auto &buff : txbuffs) {
         qvi_bbuff_free(&buff);