Merge pull request #5204 from markalle/info_snprintf

fix info-subscribe to use snprintf() and warn on long key

ompi/mpi/c/info_set.c

@@ -12,6 +12,7 @@
  * Copyright (c) 2012-2013 Inria.  All rights reserved.
  * Copyright (c) 2015      Research Organization for Information Science
  *                         and Technology (RIST). All rights reserved.
+ * Copyright (c) 2018 IBM Corporation. All rights reserved.
  * $COPYRIGHT$
  *
  * Additional copyrights may follow
@@ -26,6 +27,7 @@
 #include "ompi/communicator/communicator.h"
 #include "ompi/errhandler/errhandler.h"
 #include "ompi/info/info.h"
+#include "opal/util/show_help.h"
 #include <stdlib.h>
 #include <string.h>
 
@@ -97,6 +99,17 @@ int MPI_Info_set(MPI_Info info, const char *key, const char *value)
         }
     }
 
+// An extra warning condition is a key that uses our reserved prefix "__IN_".
+// That one is used internally to deal with the dynamic nature the key/val
+// pairs where we have callbacks that modify the val, and the MPI standard
+// wants the get_info call to give back the original setting rather than
+// the callback-modified setting. So if a user directly used a key __IN_foo
+// it would confuse our accounting slightly.
+    if (0 == strncmp(key, OPAL_INFO_SAVE_PREFIX, strlen(OPAL_INFO_SAVE_PREFIX))) {
+        opal_show_help("help-mpi-api.txt", "info-set-with-reserved-prefix", true,
+            key, OPAL_INFO_SAVE_PREFIX);
+    }
+
     OPAL_CR_ENTER_LIBRARY();
 
     /*

ompi/mpi/help-mpi-api.txt

@@ -3,6 +3,7 @@
 # Copyright (c) 2006      High Performance Computing Center Stuttgart,
 #                         University of Stuttgart.  All rights reserved.
 # Copyright (c) 2006-2015 Cisco Systems, Inc.  All rights reserved.
+# Copyright (c) 2018 IBM Corporation. All rights reserved.
 # $COPYRIGHT$
 #
 # Additional copyrights may follow
@@ -25,3 +26,8 @@ this environment.
 
   MPI function: %s
   Reason:       %s
+[info-set-with-reserved-prefix]
+Comments
+MPI_Info_set warning, key is using a reserved prefix.
+  Key:             %s
+  Reserved prefix: %s

opal/util/info.c

@@ -16,7 +16,7 @@
  *                         reserved.
  * Copyright (c) 2015-2017 Research Organization for Information Science
  *                         and Technology (RIST). All rights reserved.
- * Copyright (c) 2016-2017 IBM Corporation. All rights reserved.
+ * Copyright (c) 2016-2018 IBM Corporation. All rights reserved.
  * Copyright (c) 2017      Intel, Inc. All rights reserved.
  * $COPYRIGHT$
  *
@@ -191,7 +191,9 @@ int opal_info_dup_mode (opal_info_t *info, opal_info_t **newinfo,
          exists_IN_key = 0;
          exists_reg_key = 0;
          pkey = iterator->ie_key;
-         if (0 == strncmp(iterator->ie_key, "__IN_", 5)) {
+         if (0 == strncmp(iterator->ie_key, OPAL_INFO_SAVE_PREFIX,
+             strlen(OPAL_INFO_SAVE_PREFIX)))
+        {
              pkey += 5;
 
              is_IN_key = 1;
@@ -206,7 +208,9 @@ int opal_info_dup_mode (opal_info_t *info, opal_info_t **newinfo,
 
 // see if there is an __IN_<key> for the current <key>
              if (strlen(iterator->ie_key) + 5 < OPAL_MAX_INFO_KEY) {
-                 sprintf(savedkey, "__IN_%s", iterator->ie_key);
+                 snprintf(savedkey, OPAL_MAX_INFO_KEY,
+                     OPAL_INFO_SAVE_PREFIX "%s", iterator->ie_key);
+// (the prefix macro is a string, so the unreadable part above is a string concatenation)
                  opal_info_get_nolock (info, savedkey, OPAL_MAX_INFO_VAL,
                                        savedval, &flag);
              } else {

opal/util/info.h

@@ -14,7 +14,7 @@
  * Copyright (c) 2009      Sun Microsystems, Inc.  All rights reserved.
  * Copyright (c) 2012-2017 Los Alamos National Security, LLC. All rights
  *                         reserved.
- * Copyright (c) 2017      IBM Corporation. All rights reserved.
+ * Copyright (c) 2017-2018 IBM Corporation. All rights reserved.
  * $COPYRIGHT$
  *
  * Additional copyrights may follow
@@ -107,6 +107,10 @@ int opal_mpiinfo_init(void*);
  */
 int opal_info_dup (opal_info_t *info, opal_info_t **newinfo);
 
+// Comments might still say __IN_<key>, but the code should be using the
+// below macro instead.
+#define OPAL_INFO_SAVE_PREFIX "_OMPI_IN_"
+
 /**
  *   opal_info_dup_mpistandard - Duplicate an 'MPI_Info' object
  *

opal/util/info_subscriber.c

@@ -16,7 +16,7 @@
  *                         reserved.
  * Copyright (c) 2015      Research Organization for Information Science
  *                         and Technology (RIST). All rights reserved.
- * Copyright (c) 2016-2017 IBM Corporation. All rights reserved.
+ * Copyright (c) 2016-2018 IBM Corporation. All rights reserved.
  * Copyright (c) 2017      Intel, Inc. All rights reserved.
  * $COPYRIGHT$
  *
@@ -259,9 +259,10 @@ save_original_key_val(opal_info_t *info, char *key, char *val, int overwrite)
 
     // Checking strlen, even though it should be unnecessary.
     // This should only happen on predefined keys with short lengths.
-    if (strlen(key) + 5 < OPAL_MAX_INFO_KEY) {
-        sprintf(modkey, "__IN_%s", key);
-
+    if (strlen(key) + strlen(OPAL_INFO_SAVE_PREFIX) < OPAL_MAX_INFO_KEY) {
+        snprintf(modkey, OPAL_MAX_INFO_KEY,
+            OPAL_INFO_SAVE_PREFIX "%s", key);
+// (the prefix macro is a string, so the unreadable part above is a string concatenation)
         flag = 0;
         opal_info_get(info, modkey, 0, NULL, &flag);
         if (!flag || overwrite) {
@@ -348,6 +349,15 @@ int opal_infosubscribe_subscribe(opal_infosubscriber_t *object, char *key, char
     opal_hash_table_t *table = &object->s_subscriber_table;
     opal_callback_list_item_t *callback_list_item;
 
+    if (strlen(key) > OPAL_MAX_INFO_KEY-strlen(OPAL_INFO_SAVE_PREFIX)) {
+        fprintf(stderr, "DEVELOPER WARNING: Unexpected key length [%s]: "
+            "OMPI internal callback keys are limited to %d chars\n",
+            key, OPAL_MAX_INFO_KEY-strlen(OPAL_INFO_SAVE_PREFIX));
+#if OPAL_ENABLE_DEBUG
+        assert(!(strlen(key) > OPAL_MAX_INFO_KEY-strlen(OPAL_INFO_SAVE_PREFIX)));
+#endif
+    }
+
     if (table) {
         opal_hash_table_get_value_ptr(table, key, strlen(key), (void**) &list);