Merge pull request open-mpi#9903 from bwbarrett/cleanup/warnings-suck

bwbarrett · web-flow · commit 053ce6669b07 · 2022-01-24T08:37:27.000-08:00
Another round of warning cleanups on master
diff --git a/ompi/attribute/attribute.c b/ompi/attribute/attribute.c
@@ -594,7 +594,9 @@ static void attr_subsys_construct(attr_subsys_t *subsys)
     opal_bitmap_set_max_size (subsys->key_bitmap, 
                               OMPI_FORTRAN_HANDLE_MAX);
     ret = opal_bitmap_init(subsys->key_bitmap, 32);
-    assert(OPAL_SUCCESS == ret);
+    if (OPAL_SUCCESS != ret) {
+        abort();
+    }
 
     for (int i = 0; i < MPI_ATTR_PREDEFINED_KEY_MAX; i++) {
         opal_bitmap_set_bit(subsys->key_bitmap, i);
@@ -615,9 +617,11 @@ static void attr_subsys_construct(attr_subsys_t *subsys)
     }
 
     ret = opal_hash_table_init(subsys->keyval_hash, ATTR_TABLE_SIZE);
-    assert (OPAL_SUCCESS == ret);
+    if (OPAL_SUCCESS != ret) {
+        abort();
+    }
 
-    attr_sequence = 0;    
+    attr_sequence = 0;
 }
 
 
diff --git a/ompi/dpm/dpm.c b/ompi/dpm/dpm.c
@@ -783,8 +783,9 @@ static int dpm_convert(opal_list_t *infos,
     /**** Get here if the specified option is not found in the
      **** current list - add it
      ****/
-
-    if (NULL == directive) {
+    if (NULL == directive && NULL == modifier) {
+        return OMPI_ERR_BAD_PARAM;
+    } else if (NULL == directive) {
         opal_asprintf(&ptr, ":%s", modifier);
     } else if (NULL == modifier) {
         ptr = strdup(directive);
diff --git a/ompi/mca/coll/ftagree/coll_ftagree_earlyreturning.c b/ompi/mca/coll/ftagree/coll_ftagree_earlyreturning.c
@@ -1623,11 +1623,12 @@ static void era_decide(era_value_t *decided_value, era_agreement_info_t *ci)
     ompi_communicator_t *comm;
     era_rank_item_t *rl;
     int r, s, dead_size;
-    void *value;
 
     assert( 0 != ci->agreement_id.ERAID_FIELDS.agreementid );
 
 #if OPAL_ENABLE_DEBUG
+    void *value;
+
     r = era_parent(ci);
     if( opal_hash_table_get_value_uint64(&era_passed_agreements,
                                          ci->agreement_id.ERAID_KEY, &value) == OMPI_SUCCESS ) {
@@ -2636,7 +2637,6 @@ static void era_cb_fn(struct mca_btl_base_module_t* btl,
                       const mca_btl_base_receive_descriptor_t* descriptor)
 {
     era_incomplete_msg_t *incomplete_msg = NULL;
-    mca_btl_base_tag_t tag = descriptor->tag;
     era_msg_header_t *msg_header;
     era_frag_t *frag;
     uint64_t src_hash;
@@ -2646,7 +2646,7 @@ static void era_cb_fn(struct mca_btl_base_module_t* btl,
     int *new_dead;
     int *ack_failed;
 
-    assert(MCA_BTL_TAG_FT_AGREE == tag);
+    assert(MCA_BTL_TAG_FT_AGREE == descriptor->tag);
     assert(1 == descriptor->des_segment_count);
 
     frag = (era_frag_t*)descriptor->des_segments->seg_addr.pval;
diff --git a/ompi/mca/hook/comm_method/hook_comm_method_fns.c b/ompi/mca/hook/comm_method/hook_comm_method_fns.c
@@ -545,13 +545,17 @@ ompi_report_comm_methods(int called_from_location) // 1 = from init, 2 = from fi
             fp = fopen(mca_hook_comm_method_fakefile, "r");
             for (i=0; i<nleaderranks; ++i) {
                 for (k=0; k<nleaderranks; ++k) {
-                    fscanf(fp, "%d", &setting);
+                    if (fscanf(fp, "%d", &setting) != 1) {
+                        break;
+                    }
                     // let -1 mean "use existing (real) setting"
                     if (setting != -1) {
                         method[i * nleaderranks + k] = setting;
                     }
                 }
-                fscanf(fp, "\n");
+                if (fscanf(fp, "\n") != 0) {
+                    break;
+                }
             }
             fclose(fp);
         }
diff --git a/ompi/mca/sharedfp/sm/sharedfp_sm.c b/ompi/mca/sharedfp/sm/sharedfp_sm.c
@@ -28,6 +28,7 @@
 
 #include "ompi_config.h"
 #include "mpi.h"
+#include "opal/util/printf.h"
 #include "ompi/mca/sharedfp/sharedfp.h"
 #include "ompi/mca/sharedfp/base/base.h"
 #include "ompi/mca/sharedfp/sm/sharedfp_sm.h"
@@ -105,7 +106,7 @@ struct mca_sharedfp_base_module_1_0_0_t * mca_sharedfp_sm_component_file_query(o
     int comm_cid = -1;
     int pid = ompi_comm_rank (comm);
     
-    asprintf(&sm_filename, "%s/%s_cid-%d-%d.sm", ompi_process_info.job_session_dir,
+    opal_asprintf(&sm_filename, "%s/%s_cid-%d-%d.sm", ompi_process_info.job_session_dir,
              filename_basename, comm_cid, pid);
     free(filename_basename);
 
diff --git a/ompi/mca/sharedfp/sm/sharedfp_sm_file_open.c b/ompi/mca/sharedfp/sm/sharedfp_sm_file_open.c
@@ -35,6 +35,8 @@
 #include "sharedfp_sm.h"
 
 #include "mpi.h"
+#include "opal/util/printf.h"
+#include "opal/util/output.h"
 #include "ompi/constants.h"
 #include "ompi/group/group.h"
 #include "ompi/proc/proc.h"
@@ -119,8 +121,8 @@ int mca_sharedfp_sm_file_open (struct ompi_communicator_t *comm,
         return err;
     }
 
-    asprintf(&sm_filename, "%s/%s_cid-%d-%d.sm", ompi_process_info.job_session_dir,
-             filename_basename, comm_cid, int_pid);
+    opal_asprintf(&sm_filename, "%s/%s_cid-%d-%d.sm", ompi_process_info.job_session_dir,
+                  filename_basename, comm_cid, int_pid);
     /* open shared memory file, initialize to 0, map into memory */
     sm_fd = open(sm_filename, O_RDWR | O_CREAT,
                  S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
@@ -139,7 +141,10 @@ int mca_sharedfp_sm_file_open (struct ompi_communicator_t *comm,
     /* TODO: is it necessary to write to the file first? */
     if( 0 == fh->f_rank ){
         memset ( &sm_offset, 0, sizeof (struct mca_sharedfp_sm_offset ));
-        write ( sm_fd, &sm_offset, sizeof(struct mca_sharedfp_sm_offset));
+        err = opal_best_effort_write ( sm_fd, &sm_offset, sizeof(struct mca_sharedfp_sm_offset));
+        if (OPAL_SUCCESS != err) {
+            return err;
+        }
     }
     err = comm->c_coll->coll_barrier (comm, comm->c_coll->coll_barrier_module );
     if ( OMPI_SUCCESS != err ) {
diff --git a/ompi/runtime/ompi_mpi_init.c b/ompi/runtime/ompi_mpi_init.c
@@ -402,7 +402,9 @@ int ompi_mpi_init(int argc, char **argv, int requested, int *provided,
     ompi_proc_t** procs;
     size_t nprocs;
     char *error = NULL;
+#if OPAL_USING_INTERNAL_PMIX
     char *evar;
+#endif
     volatile bool active;
     bool background_fence = false;
     pmix_info_t info[2];
diff --git a/ompi/tools/mpirun/main.c b/ompi/tools/mpirun/main.c
@@ -23,10 +23,14 @@
 #include "opal/util/os_dirpath.h"
 #include "opal/util/os_path.h"
 #include "opal/util/path.h"
+#include "opal/util/printf.h"
 
 int main(int argc, char *argv[])
 {
-    char *evar, *pvar;
+    char *evar;
+#if OPAL_USING_INTERNAL_PMIX || OMPI_USING_INTERNAL_PRRTE
+    char *pvar;
+#endif
     char **pargs = NULL;
     char *pfx = NULL;
     int m, param_len;
@@ -35,12 +39,12 @@ int main(int argc, char *argv[])
     if (NULL != (evar = getenv("OPAL_PREFIX"))) {
 
 #if OMPI_USING_INTERNAL_PRRTE
-        (void)asprintf(&pvar, "PRTE_PREFIX=%s", evar);
+        opal_asprintf(&pvar, "PRTE_PREFIX=%s", evar);
         putenv(pvar);
 #endif
 
 #if OPAL_USING_INTERNAL_PMIX
-        (void)asprintf(&pvar, "PMIX_PREFIX=%s", evar);
+        opal_asprintf(&pvar, "PMIX_PREFIX=%s", evar);
         putenv(pvar);
 #endif
     }
@@ -51,7 +55,7 @@ int main(int argc, char *argv[])
         opal_argv_append_nosize(&pargs, argv[m]);
         /* Did the user specify a prefix, or want prefix by default? */
         if (0 == strcmp(argv[m], "--prefix")) {
-            asprintf(&pfx, "%s%s", argv[m+1], "/bin");
+            opal_asprintf(&pfx, "%s%s", argv[m+1], "/bin");
         }
     }
 
diff --git a/opal/datatype/opal_datatype_unpack.c b/opal/datatype/opal_datatype_unpack.c
@@ -547,7 +547,6 @@ int32_t opal_unpack_general_function(opal_convertor_t *pConvertor, struct iovec
     dt_stack_t *pStack; /* pointer to the position on the stack */
     uint32_t pos_desc;  /* actual position in the description of the derived datatype */
     size_t count_desc;  /* the number of items already done in the actual pos_desc */
-    uint16_t type = OPAL_DATATYPE_MAX_PREDEFINED; /* type at current position */
     size_t total_unpacked = 0;                    /* total size unpacked this time */
     dt_elem_desc_t *description;
     dt_elem_desc_t *pElem;
@@ -588,15 +587,14 @@ int32_t opal_unpack_general_function(opal_convertor_t *pConvertor, struct iovec
         while (1) {
             while (pElem->elem.common.flags & OPAL_DATATYPE_FLAG_DATA) {
                 /* now here we have a basic datatype */
-                type = description[pos_desc].elem.common.type;
                 OPAL_DATATYPE_SAFEGUARD_POINTER(conv_ptr + pElem->elem.disp, pData->size,
                                                 pConvertor->pBaseBuf, pData, pConvertor->count);
                 DO_DEBUG(opal_output(0,
                                      "unpack (%p, %ld) -> (%p:%ld, %" PRIsize_t ", %ld) type %s\n",
                                      (void *) iov_ptr, iov_len_local, (void *) pConvertor->pBaseBuf,
                                      conv_ptr + pElem->elem.disp - pConvertor->pBaseBuf, count_desc,
                                      description[pos_desc].elem.extent,
-                                     opal_datatype_basicDatatypes[type]->name););
+                                     opal_datatype_basicDatatypes[description[pos_desc].elem.common.type]->name););
                 unpack_predefined_heterogeneous(pConvertor, pElem, &count_desc, &conv_ptr, &iov_ptr,
                                                 &iov_len_local);
                 if (0 == count_desc) {    /* completed */
diff --git a/opal/mca/backtrace/execinfo/backtrace_execinfo.c b/opal/mca/backtrace/execinfo/backtrace_execinfo.c
@@ -30,6 +30,7 @@
 #endif
 
 #include "opal/constants.h"
+#include "opal/util/output.h"
 #include "opal/mca/backtrace/backtrace.h"
 
 int opal_backtrace_print(FILE *file, char *prefix, int strip)
@@ -52,10 +53,10 @@ int opal_backtrace_print(FILE *file, char *prefix, int strip)
 
     for (i = strip; i < trace_size; i++) {
         if (NULL != prefix) {
-            write(fd, prefix, strlen(prefix));
+            opal_best_effort_write(fd, prefix, strlen(prefix));
         }
         len = snprintf(buf, sizeof(buf), "[%2d] ", i - strip);
-        write(fd, buf, len);
+        opal_best_effort_write(fd, buf, len);
         backtrace_symbols_fd(&trace[i], 1, fd);
     }
 
diff --git a/opal/mca/btl/base/btl_base_am_rdma.c b/opal/mca/btl/base/btl_base_am_rdma.c
@@ -176,9 +176,6 @@ struct mca_btl_base_rdma_hdr_t {
     /* the following fields are not used on the target and are only relevant
      * to the initiator */
     uint64_t context;
-
-    /* registration handles (if required) */
-    uint8_t handle_data[];
 };
 typedef struct mca_btl_base_rdma_hdr_t mca_btl_base_rdma_hdr_t;
 
@@ -218,11 +215,6 @@ static inline size_t size_t_min(size_t a, size_t b)
     return (a < b) ? a : b;
 }
 
-static inline size_t size_t_max(size_t a, size_t b)
-{
-    return (a > b) ? a : b;
-}
-
 static mca_btl_base_am_rdma_module_t default_module;
 
 static inline bool mca_btl_base_rdma_use_rdma_get(mca_btl_base_module_t *btl)
@@ -1009,8 +1001,10 @@ static void mca_btl_base_am_process_atomic(mca_btl_base_module_t *btl,
                                                    &tmp, (int32_t) hdr->data.atomic.operand[1]);
             atomic_response = tmp;
         } else if (8 == hdr->data.atomic.size) {
+            int64_t tmp = (int64_t) atomic_response;
             opal_atomic_compare_exchange_strong_64((opal_atomic_int64_t *) hdr->target_address,
-                                                   &atomic_response, hdr->data.atomic.operand[1]);
+                                                   &tmp, hdr->data.atomic.operand[1]);
+            atomic_response = tmp;
         }
         break;
     default:
diff --git a/opal/mca/mpool/base/mpool_base_default.c b/opal/mca/mpool/base/mpool_base_default.c
@@ -34,7 +34,12 @@ static void *mca_mpool_default_alloc(mca_mpool_base_module_t *mpool, size_t size
     if (align <= sizeof(void *)) {
         addr = malloc(size);
     } else {
-        (void) posix_memalign(&addr, align, size);
+        int ret = posix_memalign(&addr, align, size);
+        if (ret < 0) {
+            /* old systems may not guarantee that addr wasn't modified
+               on failure */
+            addr = NULL;
+        }
     }
     return addr;
 #else
diff --git a/opal/mca/rcache/base/rcache_base_mem_cb.c b/opal/mca/rcache/base/rcache_base_mem_cb.c
@@ -30,6 +30,7 @@
 #endif
 
 #include "opal/runtime/opal_params.h"
+#include "opal/util/output.h"
 #include "opal/util/proc.h"
 #include "opal/util/show_help.h"
 
@@ -72,7 +73,7 @@ void mca_rcache_base_mem_cb(void *base, size_t size, void *cbdata, bool from_all
                         "will now abort.\n",
                         opal_process_info.nodename, getpid(), base, (unsigned long) size);
                     msg[sizeof(msg) - 1] = '\0';
-                    write(2, msg, len);
+                    opal_best_effort_write(2, msg, len);
                 } else {
                     opal_show_help("help-rcache-base.txt", "cannot deregister in-use memory", true,
                                    current->rcache_component->rcache_version.mca_component_name,
diff --git a/opal/util/Makefile.am b/opal/util/Makefile.am
@@ -51,7 +51,6 @@ headers = \
         clock_gettime.h \
         cmd_line.h \
         crc.h \
-        daemon_init.h \
 	ethtool.h \
         error.h \
 	event.h \
@@ -92,7 +91,6 @@ libopalutil_la_SOURCES = \
 	bipartite_graph.c \
         cmd_line.c \
         crc.c \
-        daemon_init.c \
 	ethtool.c \
         error.c \
 	event.c \
diff --git a/opal/util/daemon_init.c b/opal/util/daemon_init.c
diff --git a/opal/util/daemon_init.h b/opal/util/daemon_init.h
diff --git a/opal/util/error.c b/opal/util/error.c
diff --git a/opal/util/output.c b/opal/util/output.c
diff --git a/opal/util/output.h b/opal/util/output.h
diff --git a/opal/util/path.c b/opal/util/path.c
diff --git a/opal/util/stacktrace.c b/opal/util/stacktrace.c

Original file line number	Diff line number	Diff line change
`@@ -545,13 +545,17 @@ ompi_report_comm_methods(int called_from_location) // 1 = from init, 2 = from fi`
`545`	`545`	`fp = fopen(mca_hook_comm_method_fakefile, "r");`
`546`	`546`	`for (i=0; i<nleaderranks; ++i) {`
`547`	`547`	`for (k=0; k<nleaderranks; ++k) {`
`548`		`- fscanf(fp, "%d", &setting);`
	`548`	`+ if (fscanf(fp, "%d", &setting) != 1) {`
	`549`	`+ break;`
	`550`	`+ }`
`549`	`551`	`// let -1 mean "use existing (real) setting"`
`550`	`552`	`if (setting != -1) {`
`551`	`553`	`method[i * nleaderranks + k] = setting;`
`552`	`554`	`}`
`553`	`555`	`}`
`554`		`- fscanf(fp, "\n");`
	`556`	`+ if (fscanf(fp, "\n") != 0) {`
	`557`	`+ break;`
	`558`	`+ }`
`555`	`559`	`}`
`556`	`560`	`fclose(fp);`
`557`	`561`	`}`