Skip to content

Commit 1ef5034

Browse files
gpaulsengpaulsen
authored
Merge pull request open-mpi#9720 from awlauria/strncat_cleanup
Strncat cleanup
2 parents f33221f + 48a71c1 commit 1ef5034

File tree

6 files changed

+50
-53
lines changed

6 files changed

+50
-53
lines changed

‎ompi/errhandler/errhandler_predefined.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -403,11 +403,11 @@ static void backend_abort_no_aggregate(int fatal, char *type,
403403

404404
len -= strlen(type);
405405
if (len > 0) {
406-
strncat(str, " ", len);
406+
strncat(str, " ", len - 1);
407407

408408
--len;
409409
if (len > 0) {
410-
strncat(str, name, len);
410+
strncat(str, name, len - 1);
411411
}
412412
}
413413
out("*** on %s", str);

‎ompi/mpiext/affinity/c/mpiext_affinity_c.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
*
1212
*/
1313

14-
#define OMPI_AFFINITY_STRING_MAX 1024
14+
#define OMPI_AFFINITY_STRING_MAX BUFSIZ
1515

1616
typedef enum ompi_affinity_fmt {
1717
OMPI_AFFINITY_RSRC_STRING_FMT,

‎ompi/mpiext/affinity/c/mpiext_affinity_str.c‎

Lines changed: 25 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -177,11 +177,9 @@ static char *bitmap2rangestr(int bitmap)
177177
size_t i;
178178
int range_start, range_end;
179179
bool first, isset;
180-
char tmp[BUFSIZ];
180+
char tmp[OMPI_AFFINITY_STRING_MAX - 1] = {0};
181181
const int stmp = sizeof(tmp) - 1;
182-
static char ret[BUFSIZ];
183-
184-
memset(ret, 0, sizeof(ret));
182+
static char ret[OMPI_AFFINITY_STRING_MAX] = {0};
185183

186184
first = true;
187185
range_start = -999;
@@ -207,7 +205,7 @@ static char *bitmap2rangestr(int bitmap)
207205
snprintf(tmp, stmp, "%d-%d", range_start, range_end);
208206
}
209207
size_t ret_len = strlen(ret);
210-
snprintf(ret + ret_len, BUFSIZ - ret_len, "%s", tmp);
208+
snprintf(ret + ret_len, sizeof(ret) - ret_len, "%s", tmp);
211209

212210
range_start = -999;
213211
}
@@ -235,7 +233,7 @@ static char *bitmap2rangestr(int bitmap)
235233
snprintf(tmp, stmp, "%d-%d", range_start, range_end);
236234
}
237235
size_t ret_len = strlen(ret);
238-
snprintf(ret + ret_len, BUFSIZ - ret_len, "%s", tmp);
236+
snprintf(ret + ret_len, sizeof(ret) - ret_len, "%s", tmp);
239237
}
240238

241239
return ret;
@@ -249,7 +247,7 @@ static int cset2str(char *str, int len, hwloc_topology_t topo, hwloc_cpuset_t cp
249247
bool first;
250248
int num_sockets, num_cores;
251249
int ret, socket_index, core_index;
252-
char tmp[BUFSIZ];
250+
char tmp[OMPI_AFFINITY_STRING_MAX - 1] = {0};
253251
const int stmp = sizeof(tmp) - 1;
254252
int **map = NULL;
255253

@@ -350,7 +348,7 @@ static bool is_single_cpu(hwloc_cpuset_t cpuset)
350348
*/
351349
static int cset2mapstr(char *str, int len, hwloc_topology_t topo, hwloc_cpuset_t cpuset)
352350
{
353-
char tmp[BUFSIZ];
351+
char tmp[OMPI_AFFINITY_STRING_MAX - 1] = {0};
354352
int core_index, pu_index;
355353
const int stmp = sizeof(tmp) - 1;
356354
hwloc_obj_t socket, core, pu;
@@ -432,7 +430,7 @@ static int get_rsrc_current_binding(char str[OMPI_AFFINITY_STRING_MAX])
432430

433431
/* If we are not bound, indicate that */
434432
if (!bound) {
435-
strncat(str, not_bound_str, OMPI_AFFINITY_STRING_MAX - 1);
433+
strncat(str, not_bound_str, OMPI_AFFINITY_STRING_MAX - strlen(str) - 1);
436434
ret = OMPI_SUCCESS;
437435
}
438436

@@ -461,7 +459,7 @@ static int get_rsrc_exists(char str[OMPI_AFFINITY_STRING_MAX])
461459
{
462460
bool first = true;
463461
int i, num_cores, num_pus;
464-
char tmp[OMPI_AFFINITY_STRING_MAX];
462+
char tmp[OMPI_AFFINITY_STRING_MAX - 1] = {0};
465463
const int stmp = sizeof(tmp) - 1;
466464
hwloc_obj_t socket, core, c2;
467465

@@ -471,12 +469,12 @@ static int get_rsrc_exists(char str[OMPI_AFFINITY_STRING_MAX])
471469
NULL != socket; socket = socket->next_cousin) {
472470
/* If this isn't the first socket, add a delimiter */
473471
if (!first) {
474-
strncat(str, "; ", OMPI_AFFINITY_STRING_MAX - strlen(str));
472+
strncat(str, "; ", OMPI_AFFINITY_STRING_MAX - strlen(str) - 1);
475473
}
476474
first = false;
477475

478476
snprintf(tmp, stmp, "socket %d has ", socket->os_index);
479-
strncat(str, tmp, OMPI_AFFINITY_STRING_MAX - strlen(str));
477+
strncat(str, tmp, OMPI_AFFINITY_STRING_MAX - strlen(str) - 1);
480478

481479
/* Find out how many cores are inside this socket, and get an
482480
object pointing to the first core. Also count how many PUs
@@ -496,13 +494,13 @@ static int get_rsrc_exists(char str[OMPI_AFFINITY_STRING_MAX])
496494
/* Only 1 core */
497495
if (1 == num_cores) {
498496
strncat(str, "1 core with ",
499-
OMPI_AFFINITY_STRING_MAX - strlen(str));
497+
OMPI_AFFINITY_STRING_MAX - strlen(str) - 1);
500498
if (1 == num_pus) {
501499
strncat(str, "1 hwt",
502-
OMPI_AFFINITY_STRING_MAX - strlen(str));
500+
OMPI_AFFINITY_STRING_MAX - strlen(str) - 1);
503501
} else {
504502
snprintf(tmp, stmp, "%d hwts", num_pus);
505-
strncat(str, tmp, OMPI_AFFINITY_STRING_MAX - strlen(str));
503+
strncat(str, tmp, OMPI_AFFINITY_STRING_MAX - strlen(str) - 1);
506504
}
507505
}
508506

@@ -511,7 +509,7 @@ static int get_rsrc_exists(char str[OMPI_AFFINITY_STRING_MAX])
511509
bool same = true;
512510

513511
snprintf(tmp, stmp, "%d cores", num_cores);
514-
strncat(str, tmp, OMPI_AFFINITY_STRING_MAX - strlen(str));
512+
strncat(str, tmp, OMPI_AFFINITY_STRING_MAX - strlen(str) - 1);
515513

516514
/* Do all the cores have the same number of PUs? */
517515
for (c2 = core; NULL != c2; c2 = c2->next_cousin) {
@@ -527,32 +525,32 @@ static int get_rsrc_exists(char str[OMPI_AFFINITY_STRING_MAX])
527525
/* Yes, they all have the same number of PUs */
528526
if (same) {
529527
snprintf(tmp, stmp, ", each with %d hwt", num_pus);
530-
strncat(str, tmp, OMPI_AFFINITY_STRING_MAX - strlen(str));
528+
strncat(str, tmp, OMPI_AFFINITY_STRING_MAX - strlen(str) - 1);
531529
if (num_pus != 1) {
532-
strncat(str, "s", OMPI_AFFINITY_STRING_MAX - strlen(str));
530+
strncat(str, "s", OMPI_AFFINITY_STRING_MAX - strlen(str) - 1);
533531
}
534532
}
535533

536534
/* No, they have differing numbers of PUs */
537535
else {
538536
bool first_iter = true;
539537

540-
strncat(str, "with (", OMPI_AFFINITY_STRING_MAX - strlen(str));
538+
strncat(str, "with (", OMPI_AFFINITY_STRING_MAX - strlen(str) - 1);
541539
for (c2 = core; NULL != c2; c2 = c2->next_cousin) {
542540
if (!first_iter) {
543541
strncat(str, ", ",
544-
OMPI_AFFINITY_STRING_MAX - strlen(str));
542+
OMPI_AFFINITY_STRING_MAX - strlen(str) - 1);
545543
}
546544
first_iter = false;
547545

548546
i = hwloc_get_nbobjs_inside_cpuset_by_type(opal_hwloc_topology,
549547
core->cpuset,
550548
HWLOC_OBJ_PU);
551549
snprintf(tmp, stmp, "%d", i);
552-
strncat(str, tmp, OMPI_AFFINITY_STRING_MAX - strlen(str));
550+
strncat(str, tmp, OMPI_AFFINITY_STRING_MAX - strlen(str) - 1);
553551
}
554552
strncat(str, ") hwts",
555-
OMPI_AFFINITY_STRING_MAX - strlen(str));
553+
OMPI_AFFINITY_STRING_MAX - strlen(str) - 1);
556554
}
557555
}
558556
}
@@ -620,7 +618,7 @@ static int get_layout_current_binding(char str[OMPI_AFFINITY_STRING_MAX])
620618

621619
/* If we are not bound, indicate that */
622620
if (!bound) {
623-
strncat(str, not_bound_str, OMPI_AFFINITY_STRING_MAX - 1);
621+
strncat(str, not_bound_str, OMPI_AFFINITY_STRING_MAX - strlen(str) - 1);
624622
ret = OMPI_SUCCESS;
625623
}
626624

@@ -652,7 +650,6 @@ static int get_layout_current_binding(char str[OMPI_AFFINITY_STRING_MAX])
652650
static int get_layout_exists(char str[OMPI_AFFINITY_STRING_MAX])
653651
{
654652
int core_index, pu_index;
655-
int len = OMPI_AFFINITY_STRING_MAX;
656653
hwloc_obj_t socket, core, pu;
657654

658655
str[0] = '\0';
@@ -662,7 +659,7 @@ static int get_layout_exists(char str[OMPI_AFFINITY_STRING_MAX])
662659
HWLOC_OBJ_SOCKET, 0);
663660
NULL != socket;
664661
socket = socket->next_cousin) {
665-
strncat(str, "[", len - strlen(str));
662+
strncat(str, "[", OMPI_AFFINITY_STRING_MAX - strlen(str) - 1);
666663

667664
/* Iterate over all existing cores in this socket */
668665
core_index = 0;
@@ -674,7 +671,7 @@ static int get_layout_exists(char str[OMPI_AFFINITY_STRING_MAX])
674671
socket->cpuset,
675672
HWLOC_OBJ_CORE, ++core_index)) {
676673
if (core_index > 0) {
677-
strncat(str, "/", len - strlen(str));
674+
strncat(str, "/", OMPI_AFFINITY_STRING_MAX - strlen(str) - 1);
678675
}
679676

680677
/* Iterate over all existing PUs in this core */
@@ -686,10 +683,10 @@ static int get_layout_exists(char str[OMPI_AFFINITY_STRING_MAX])
686683
pu = hwloc_get_obj_inside_cpuset_by_type(opal_hwloc_topology,
687684
core->cpuset,
688685
HWLOC_OBJ_PU, ++pu_index)) {
689-
strncat(str, ".", len - strlen(str));
686+
strncat(str, ".", OMPI_AFFINITY_STRING_MAX - strlen(str) - 1);
690687
}
691688
}
692-
strncat(str, "]", len - strlen(str));
689+
strncat(str, "]", OMPI_AFFINITY_STRING_MAX - strlen(str) - 1);
693690
}
694691

695692
return OMPI_SUCCESS;

‎ompi/tools/ompi_info/param.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,10 @@ static void append(char *dest, size_t max, int *first, char *src)
6262

6363
len = max - strlen(dest);
6464
if (!(*first)) {
65-
strncat(dest, ", ", len);
65+
strncat(dest, ", ", len - 1);
6666
len = max - strlen(dest);
6767
}
68-
strncat(dest, src, len);
68+
strncat(dest, src, len - 1);
6969
*first = 0;
7070
}
7171

‎opal/util/cmd_line.c‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -499,7 +499,7 @@ char *opal_cmd_line_get_usage_msg(opal_cmd_line_t *cmd)
499499
int argc;
500500
size_t j;
501501
char **argv;
502-
char *ret, temp[(MAX_WIDTH * 2) - 1], line[MAX_WIDTH * 2];
502+
char *ret, line[(MAX_WIDTH * 2) + 1];
503503
char *start, *desc, *ptr;
504504
opal_list_item_t *item;
505505
ompi_cmd_line_option_t *option, **sorted;
@@ -559,27 +559,27 @@ char *opal_cmd_line_get_usage_msg(opal_cmd_line_t *cmd)
559559
}
560560
if (NULL != option->clo_single_dash_name) {
561561
line[2] = (filled) ? '|' : ' ';
562-
strncat(line, "-", sizeof(line) - 1);
563-
strncat(line, option->clo_single_dash_name, sizeof(line) - 1);
562+
strncat(line, "-", sizeof(line) - strlen(line) - 1);
563+
strncat(line, option->clo_single_dash_name, sizeof(line) - strlen(line) - 1);
564564
filled = true;
565565
}
566566
if (NULL != option->clo_long_name) {
567567
if (filled) {
568-
strncat(line, "|", sizeof(line) - 1);
568+
strncat(line, "|", sizeof(line) - strlen(line) - 1);
569569
} else {
570-
strncat(line, " ", sizeof(line) - 1);
570+
strncat(line, " ", sizeof(line) - strlen(line) - 1);
571571
}
572-
strncat(line, "--", sizeof(line) - 1);
573-
strncat(line, option->clo_long_name, sizeof(line) - 1);
572+
strncat(line, "--", sizeof(line) - strlen(line) - 1);
573+
strncat(line, option->clo_long_name, sizeof(line) - strlen(line) - 1);
574574
}
575-
strncat(line, " ", sizeof(line) - 1);
575+
strncat(line, " ", sizeof(line) - strlen(line) - 1);
576576
for (i = 0; (int) i < option->clo_num_params; ++i) {
577-
len = sizeof(temp);
578-
snprintf(temp, len, "<arg%d> ", (int) i);
579-
strncat(line, temp, sizeof(line) - 1);
577+
char temp[MAX_WIDTH * 2];
578+
snprintf(temp, MAX_WIDTH * 2, "<arg%d> ", (int) i);
579+
strncat(line, temp, sizeof(line) - strlen(line) - 1);
580580
}
581581
if (option->clo_num_params > 0) {
582-
strncat(line, " ", sizeof(line) - 1);
582+
strncat(line, " ", sizeof(line) - strlen(line) - 1);
583583
}
584584

585585
/* If we're less than param width, then start adding the
@@ -635,7 +635,7 @@ char *opal_cmd_line_get_usage_msg(opal_cmd_line_t *cmd)
635635
/* Last line */
636636

637637
if (strlen(start) < (MAX_WIDTH - PARAM_WIDTH)) {
638-
strncat(line, start, sizeof(line) - 1);
638+
strncat(line, start, sizeof(line) - strlen(line) - 1);
639639
opal_argv_append(&argc, &argv, line);
640640
break;
641641
}
@@ -647,7 +647,7 @@ char *opal_cmd_line_get_usage_msg(opal_cmd_line_t *cmd)
647647
for (ptr = start + (MAX_WIDTH - PARAM_WIDTH); ptr > start; --ptr) {
648648
if (isspace(*ptr)) {
649649
*ptr = '\0';
650-
strncat(line, start, sizeof(line) - 1);
650+
strncat(line, start, sizeof(line) - strlen(line) - 1);
651651
opal_argv_append(&argc, &argv, line);
652652

653653
start = ptr + 1;
@@ -666,7 +666,7 @@ char *opal_cmd_line_get_usage_msg(opal_cmd_line_t *cmd)
666666
if (isspace(*ptr)) {
667667
*ptr = '\0';
668668

669-
strncat(line, start, sizeof(line) - 1);
669+
strncat(line, start, sizeof(line) - strlen(line) - 1);
670670
opal_argv_append(&argc, &argv, line);
671671

672672
start = ptr + 1;
@@ -680,7 +680,7 @@ char *opal_cmd_line_get_usage_msg(opal_cmd_line_t *cmd)
680680
whitespace, then just add it on and be done */
681681

682682
if (ptr >= start + len) {
683-
strncat(line, start, sizeof(line) - 1);
683+
strncat(line, start, sizeof(line) - strlen(line) - 1);
684684
opal_argv_append(&argc, &argv, line);
685685
start = desc + len + 1;
686686
}

‎opal/util/os_path.c‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -88,15 +88,15 @@ char *opal_os_path(int relative, ...)
8888
va_start(ap, relative);
8989
if (NULL != (element = va_arg(ap, char *))) {
9090
if (path_sep[0] != element[0]) {
91-
strncat(path, path_sep, total_length);
91+
strncat(path, path_sep, total_length - strlen(path) - 1);
9292
}
93-
strcat(path, element);
93+
strncat(path, element, total_length - strlen(path) - 1);
9494
}
9595
while (NULL != (element = va_arg(ap, char *))) {
9696
if (path_sep[0] != element[0]) {
97-
strncat(path, path_sep, total_length);
97+
strncat(path, path_sep, total_length - strlen(path) - 1);
9898
}
99-
strncat(path, element, total_length);
99+
strncat(path, element, total_length - strlen(path) - 1);
100100
}
101101

102102
va_end(ap);

0 commit comments

Comments
 (0)