Fix MPI_Info_get to pass valuelen+1 to MPIU_Strncpy and check return

code of MPIU_Strncpy. Added test program.
Closes #2225
Signed-off-by: William Gropp <wgropp@illinois.edu>

src/include/mpiimpl.h

@@ -4418,7 +4418,7 @@ int MPIR_Cart_map_impl(const MPID_Comm *comm_ptr, int ndims, const int dims[],
                        const int periodic[], int *newrank);
 int MPIR_Close_port_impl(const char *port_name);
 int MPIR_Open_port_impl(MPID_Info *info_ptr, char *port_name);
-void MPIR_Info_get_impl(MPID_Info *info_ptr, const char *key, int valuelen, char *value, int *flag);
+int MPIR_Info_get_impl(MPID_Info *info_ptr, const char *key, int valuelen, char *value, int *flag);
 void MPIR_Info_get_nkeys_impl(MPID_Info *info_ptr, int *nkeys);
 int MPIR_Info_get_nthkey_impl(MPID_Info *info, int n, char *key);
 void MPIR_Info_get_valuelen_impl(MPID_Info *info_ptr, const char *key, int *valuelen, int *flag);

src/mpi/info/info_get.c

@@ -29,28 +29,35 @@ int MPI_Info_get(MPI_Info info, const char *key, int valuelen, char *value, int
 #define FUNCNAME MPIR_Info_get_impl
 #undef FCNAME
 #define FCNAME MPIU_QUOTE(FUNCNAME)
-void MPIR_Info_get_impl(MPID_Info *info_ptr, const char *key, int valuelen, char *value, int *flag)
+int MPIR_Info_get_impl(MPID_Info *info_ptr, const char *key, int valuelen, char *value, int *flag)
 {
     MPID_Info *curr_ptr;
+    int err=0, mpi_errno=0;
+
     curr_ptr = info_ptr->next;
     *flag = 0;
 
     while (curr_ptr) {
         if (!strncmp(curr_ptr->key, key, MPI_MAX_INFO_KEY)) {
-            MPIU_Strncpy(value, curr_ptr->value, valuelen);
-            /* The following is problematic - if the user passes the
-               declared length, then this will access memory one
-               passed that point */
-            /* FIXME: The real fix is to change MPIU_Strncpy to
-               set the null at the end (always!) and return an error
-               if it had to truncate the result. */
-            /* value[valuelen] = '\0'; */
+            err = MPIU_Strncpy(value, curr_ptr->value, valuelen+1);
+            /* +1 because the MPI Standard says "In C, valuelen
+             * (passed to MPI_Info_get) should be one less than the
+             * amount of allocated space to allow for the null
+             * terminator*/
             *flag = 1;
             break;
         }
         curr_ptr = curr_ptr->next;
     }
-    return;
+
+    /* --BEGIN ERROR HANDLING-- */
+    if (err != 0)
+    {
+        mpi_errno = MPIR_Err_create_code(MPI_SUCCESS, MPIR_ERR_RECOVERABLE, FCNAME, __LINE__, MPI_ERR_INFO_VALUE, "**infovallong", NULL);
+    }
+    /* --END ERROR HANDLING-- */
+
+    return mpi_errno;
 }
 
 #endif
@@ -61,7 +68,7 @@ void MPIR_Info_get_impl(MPID_Info *info_ptr, const char *key, int valuelen, char
 Input Parameters:
 + info - info object (handle)
 . key - key (string)
-- valuelen - length of value argument (integer)
+- valuelen - length of value argument, not including null terminator (integer)
 
 Output Parameters:
 + value - value (string)
@@ -139,8 +146,9 @@ int MPI_Info_get(MPI_Info info, const char *key, int valuelen, char *value,
 #   endif /* HAVE_ERROR_CHECKING */
 
     /* ... body of routine ...  */
-    MPIR_Info_get_impl(info_ptr, key, valuelen, value, flag);
+    mpi_errno = MPIR_Info_get_impl(info_ptr, key, valuelen, value, flag);
     /* ... end of body of routine ... */
+    if (mpi_errno) goto fn_fail;
 
 #ifdef HAVE_ERROR_CHECKING
   fn_exit:

test/mpi/info/Makefile.am

@@ -20,4 +20,5 @@ noinst_PROGRAMS = \
     infomany      \
     infomany2     \
     infotest      \
+    infoget      \
     infoenv

test/mpi/info/infoget.c

+/* -*- Mode: C; c-basic-offset:4 ; indent-tabs-mode:nil ; -*- */
+/*
+ *  (C) 2001 by Argonne National Laboratory.
+ *      See COPYRIGHT in top-level directory.
+ */
+
+/* Test code provided by Hajime Fujita. See Trac ticket #2225. */
+
+#include "mpi.h"
+#include <stdio.h>
+#include "mpitest.h"
+#include <string.h>
+
+int main(int argc, char *argv[])
+{
+    MPI_Info info;
+    const char *key = "key", *val = "val";
+    char buff[3 + 1]; /* strlen("val") + 1 */
+    int flag, errs=0;
+
+    MTest_Init(&argc, &argv);
+
+    MPI_Info_create(&info);
+    MPI_Info_set(info, key, val);
+    MPI_Info_get(info, key, sizeof(buff)-1, buff, &flag);
+    if (flag) {
+        if (strncmp(buff, val, sizeof(buff)-1) != 0) {
+            errs++;
+            printf("returned value is %s, should be %s\n", buff, val);
+        }
+    }
+    else {
+        errs++;
+        printf("key not found\n");
+    }
+    MPI_Info_free(&info);
+
+    MTest_Finalize(errs);
+    MPI_Finalize();
+
+    return 0;
+}

test/mpi/info/testlist

@@ -5,4 +5,5 @@ infoorder 1
 infomany 1
 infomany2 1
 infotest 1
+infoget 1
 infoenv 1 mpiversion=3.0