Skip to content

Commit c48e267

Browse files
committed
Cleanup use of strncat().
Make sure the length calculation takes prior calls into consideration, and make sure it will be '\0' terminated. The basic format for strncat() calls are: strncat(dest, source, dest_size-strlen(dest)-1); Signed-off-by: Austen Lauria <awlauria@us.ibm.com> (cherry picked from commit 48a71c1)
1 parent 2cd2795 commit c48e267

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)