Skip to content

Commit 6eda2ed

Browse files
hjelmnedgargabriel
authored andcommitted
fcoll/two_phase: fix coverity errors
Fixes CIDs 72300, 72344, 1196764-1196768, 72300: Resource leaks Mulitple allocated arrays are going out of scope at the end of mca_fcoll_two_phase_file_write_all. Free these arrays. Also removed the extraneous NULL checks since free (NULL) is safe in C. Change returns to goto exit where the allocated resources are freed. Fixes CIDs 72285-72292, 72297, 72298: Resource leaks Change all appropriate return statements to goto exit to ensure that all resources are freed. Also removed the NULL checks since free (NULL) is safe in C. Fixes CIDs 72295, 72296: Resource leaks Moved free of requests and recv_types to after exit label. This will ensure these are freed on error. Also added a loop and statement to free send_buf which is going out of scope at the end of the function. Fixes CIDs 72336-72240, 735197, 735198: Resource leaks Moved the exit label before to before the resources are released and changed all appropriate return statements to goto exit. Also removed extraneous NULL checks because free (NULL) is safe in C. Fixes CIDs 72341, 72343, 1196805-1196809: Resource leaks Free all resources after exit label and change return statements to goto exit to ensure all resources are freed on error. Fixes CID 1269973: Unused value Check return code of ompi_request_wait_all. If it fails jump to the exit. Fixes CID 714119: Dereference before NULL check Wrong value checked in conditional. Signed-off-by: Nathan Hjelm <[email protected]>
1 parent e6427c0 commit 6eda2ed

File tree

3 files changed

+120
-141
lines changed

3 files changed

+120
-141
lines changed

ompi/mca/fcoll/two_phase/fcoll_two_phase_file_read_all.c

Lines changed: 44 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
/* -*- Mode: C; c-basic-offset:4 ; indent-tabs-mode:nil -*- */
12
/*
23
* Copyright (c) 2004-2005 The Trustees of Indiana University and Indiana
34
* University Research and Technology
@@ -11,6 +12,8 @@
1112
* All rights reserved.
1213
* Copyright (c) 2008-2014 University of Houston. All rights reserved.
1314
* Copyright (c) 2015 Cisco Systems, Inc. All rights reserved.
15+
* Copyright (c) 2015 Los Alamos National Security, LLC. All rights
16+
* reserved.
1417
* $COPYRIGHT$
1518
*
1619
* Additional copyrights may follow
@@ -186,7 +189,7 @@ mca_fcoll_two_phase_file_read_all (mca_io_ompio_file_t *fh,
186189
two_phase_num_io_procs,
187190
max_data);
188191
if (OMPI_SUCCESS != ret){
189-
return ret;
192+
goto exit;
190193
}
191194

192195
two_phase_num_io_procs = fh->f_final_num_aggrs;
@@ -497,21 +500,19 @@ mca_fcoll_two_phase_file_read_all (mca_io_ompio_file_t *fh,
497500
if (flat_buf->indices != NULL){
498501
free (flat_buf->indices);
499502
}
500-
free(flat_buf);
501503
flat_buf = NULL;
502504
}
503-
if (start_offsets != NULL){
504-
free(start_offsets);
505-
start_offsets = NULL;
506-
}
507-
if (end_offsets != NULL){
508-
free(end_offsets);
509-
end_offsets = NULL;
510-
}
511-
if (aggregator_list != NULL){
512-
free(aggregator_list);
513-
aggregator_list = NULL;
514-
}
505+
506+
free (start_offsets);
507+
free (end_offsets);
508+
free (aggregator_list);
509+
free (fd_start);
510+
free (decoded_iov);
511+
free (buf_indices);
512+
free (count_my_req_per_proc);
513+
free (my_req);
514+
free (others_req);
515+
free (fd_end);
515516

516517
return ret;
517518
}
@@ -634,8 +635,8 @@ static int two_phase_read_and_exch(mca_io_ompio_file_t *fh,
634635

635636
start_pos = (int *) calloc(fh->f_size, sizeof(int));
636637
if ( NULL == start_pos ){
637-
ret = OMPI_ERR_OUT_OF_RESOURCE;
638-
return ret;
638+
ret = OMPI_ERR_OUT_OF_RESOURCE;
639+
goto exit;
639640
}
640641

641642
done = 0;
@@ -717,7 +718,8 @@ static int two_phase_read_and_exch(mca_io_ompio_file_t *fh,
717718
(1,sizeof(mca_io_ompio_io_array_t));
718719
if (NULL == fh->f_io_array) {
719720
opal_output(1, "OUT OF MEMORY\n");
720-
return OMPI_ERR_OUT_OF_RESOURCE;
721+
ret = OMPI_ERR_OUT_OF_RESOURCE;
722+
goto exit;
721723
}
722724
fh->f_io_array[0].offset = (IOVBASE_TYPE *)(intptr_t)off;
723725
fh->f_io_array[0].length = len;
@@ -728,7 +730,8 @@ static int two_phase_read_and_exch(mca_io_ompio_file_t *fh,
728730
if (fh->f_num_of_io_entries){
729731
if ( 0 > fh->f_fbtl->fbtl_preadv (fh)) {
730732
opal_output(1, "READ FAILED\n");
731-
return OMPI_ERROR;
733+
ret = OMPI_ERROR;
734+
goto exit;
732735
}
733736
}
734737

@@ -797,40 +800,17 @@ static int two_phase_read_and_exch(mca_io_ompio_file_t *fh,
797800
flat_buf, others_req, m, buf_idx,
798801
buftype_extent, striping_unit, two_phase_num_io_procs,
799802
aggregator_list);
800-
if (ntimes){
801-
free(read_buf);
802-
read_buf = NULL;
803-
}
804-
if (NULL != curr_offlen_ptr){
805-
free(curr_offlen_ptr);
806-
curr_offlen_ptr = NULL;
807-
}
808-
if (NULL != count){
809-
free(count);
810-
count = NULL;
811-
}
812-
if (NULL != partial_send){
813-
free(partial_send);
814-
partial_send = NULL;
815-
}
816-
if (NULL != send_size){
817-
free(send_size);
818-
send_size = NULL;
819-
}
820-
if (NULL != recv_size){
821-
free(recv_size);
822-
recv_size = NULL;
823-
}
824-
if (NULL != recd_from_proc){
825-
free(recd_from_proc);
826-
recd_from_proc = NULL;
827-
}
828-
if (NULL != start_pos){
829-
free(start_pos);
830-
start_pos = NULL;
831-
}
832803

833804
exit:
805+
free (read_buf);
806+
free (curr_offlen_ptr);
807+
free (count);
808+
free (partial_send);
809+
free (send_size);
810+
free (recv_size);
811+
free (recd_from_proc);
812+
free (start_pos);
813+
834814
return ret;
835815

836816
}
@@ -919,7 +899,7 @@ static int two_phase_exchange_data(mca_io_ompio_file_t *fh,
919899
}
920900
else{
921901

922-
recv_buf = (char **)malloc(fh->f_size * sizeof(char *));
902+
recv_buf = (char **) calloc (fh->f_size, sizeof(char *));
923903
if (NULL == recv_buf){
924904
ret = OMPI_ERR_OUT_OF_RESOURCE;
925905
goto exit;
@@ -983,7 +963,9 @@ static int two_phase_exchange_data(mca_io_ompio_file_t *fh,
983963
ret = ompi_request_wait_all(nprocs_recv,
984964
requests,
985965
MPI_STATUS_IGNORE);
986-
966+
if (OMPI_SUCCESS != ret) {
967+
goto exit;
968+
}
987969

988970
if (! (fh->f_flags & OMPIO_CONTIGUOUS_MEMORY)) {
989971

@@ -1001,26 +983,23 @@ static int two_phase_exchange_data(mca_io_ompio_file_t *fh,
1001983
requests+nprocs_recv,
1002984
MPI_STATUS_IGNORE);
1003985

1004-
if (NULL != requests){
1005-
free(requests);
1006-
requests = NULL;
1007-
}
986+
#if OMPIO_FCOLL_WANT_TIME_BREAKDOWN
987+
end_rcomm_time = MPI_Wtime();
988+
rcomm_time += (end_rcomm_time - start_rcomm_time);
989+
#endif
990+
991+
exit:
1008992

1009-
if (! (fh->f_flags & OMPIO_CONTIGUOUS_MEMORY)){
993+
if (recv_buf) {
1010994
for (i=0; i< fh->f_size; i++){
1011-
if (recv_size[i]){
1012-
free(recv_buf[i]);
1013-
}
995+
free(recv_buf[i]);
1014996
}
997+
1015998
free(recv_buf);
1016999
}
10171000

1018-
#if OMPIO_FCOLL_WANT_TIME_BREAKDOWN
1019-
end_rcomm_time = MPI_Wtime();
1020-
rcomm_time += (end_rcomm_time - start_rcomm_time);
1021-
#endif
1001+
free(requests);
10221002

1023-
exit:
10241003
return ret;
10251004

10261005
}

0 commit comments

Comments
 (0)