Fix pack/unpack of grpcomm signature

It is okay to pack/unpack a NULL string, so take the safe route and do so. Signed-off-by: Ralph Castain <[email protected]>
openpmix · Feb 29, 2024 · f67a724 · f67a724
1 parent dd386dc
commit f67a724
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 24 deletions.
diff --git a/src/runtime/data_type_support/prte_dt_packing_fns.c b/src/runtime/data_type_support/prte_dt_packing_fns.c
@@ -616,10 +616,12 @@ int prte_grpcomm_sig_pack(pmix_data_buffer_t *bkt,
         PMIX_ERROR_LOG(rc);
         return prte_pmix_convert_status(rc);
     }
-    rc = PMIx_Data_pack(NULL, bkt, sig->signature, sig->sz, PMIX_PROC);
-    if (PMIX_SUCCESS != rc) {
-        PMIX_ERROR_LOG(rc);
-        return prte_pmix_convert_status(rc);
+    if (0 < sig->sz) {
+        rc = PMIx_Data_pack(NULL, bkt, sig->signature, sig->sz, PMIX_PROC);
+        if (PMIX_SUCCESS != rc) {
+            PMIX_ERROR_LOG(rc);
+            return prte_pmix_convert_status(rc);
+        }
     }
 
     // pack the context ID, if one was given
@@ -657,13 +659,11 @@ int prte_grpcomm_sig_pack(pmix_data_buffer_t *bkt,
         return prte_pmix_convert_status(rc);
     }
 
-    if (NULL != sig->groupID) {
-        // add the groupID if one is given
-        rc = PMIx_Data_pack(NULL, bkt, &sig->groupID, 1, PMIX_STRING);
-        if (PMIX_SUCCESS != rc) {
-            PMIX_ERROR_LOG(rc);
-            return prte_pmix_convert_status(rc);
-        }
+    // add the groupID if one is given
+    rc = PMIx_Data_pack(NULL, bkt, &sig->groupID, 1, PMIX_STRING);
+    if (PMIX_SUCCESS != rc) {
+        PMIX_ERROR_LOG(rc);
+        return prte_pmix_convert_status(rc);
     }
 
     return PRTE_SUCCESS;

diff --git a/src/runtime/data_type_support/prte_dt_unpacking_fns.c b/src/runtime/data_type_support/prte_dt_unpacking_fns.c
@@ -712,7 +712,6 @@ int prte_grpcomm_sig_unpack(pmix_data_buffer_t *buffer,
     pmix_status_t rc;
     int32_t cnt;
     prte_grpcomm_signature_t *s;
-    char *save;
 
     s = PMIX_NEW(prte_grpcomm_signature_t);
 
@@ -724,13 +723,15 @@ int prte_grpcomm_sig_unpack(pmix_data_buffer_t *buffer,
         PMIX_RELEASE(s);
         return prte_pmix_convert_status(rc);
     }
-    PMIX_PROC_CREATE(s->signature, s->sz);
-    cnt = s->sz;
-    rc = PMIx_Data_unpack(NULL, buffer, s->signature, &cnt, PMIX_PROC);
-    if (PMIX_SUCCESS != rc) {
-        PMIX_ERROR_LOG(rc);
-        PMIX_RELEASE(s);
-        return prte_pmix_convert_status(rc);
+    if (0 < s->sz) {
+        PMIX_PROC_CREATE(s->signature, s->sz);
+        cnt = s->sz;
+        rc = PMIx_Data_unpack(NULL, buffer, s->signature, &cnt, PMIX_PROC);
+        if (PMIX_SUCCESS != rc) {
+            PMIX_ERROR_LOG(rc);
+            PMIX_RELEASE(s);
+            return prte_pmix_convert_status(rc);
+        }
     }
 
     // unpack the context ID, if one was assigned
@@ -779,14 +780,13 @@ int prte_grpcomm_sig_unpack(pmix_data_buffer_t *buffer,
         return prte_pmix_convert_status(rc);
     }
 
-    // try and unpack the groupID - error is okay, just means
-    // one wasn't provided
-    save = buffer->unpack_ptr;
+    // unpack the groupID
     cnt = 1;
     rc = PMIx_Data_unpack(NULL, buffer, &s->groupID, &cnt, PMIX_STRING);
-    // ignore the return code
     if (PMIX_SUCCESS != rc) {
-        buffer->unpack_ptr = save;  // reset location for next unpack
+        PMIX_ERROR_LOG(rc);
+        PMIX_RELEASE(s);
+        return prte_pmix_convert_status(rc);
     }
 
     *sig = s;