Replace sprintf with snprintf #13468
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -325,9 +325,9 @@ abbreviate_list_into_string(char *str, int max, int *list, int nlist) | |||||
| strcpy(&str[strlen(str)], ", "); | ||||||
| } | ||||||
| if (lo != hi) { | ||||||
| snprintf(&str[strlen(str)], max - strlen(str), "%d - %d", lo, hi); | ||||||
| } else { | ||||||
| snprintf(&str[strlen(str)], max - strlen(str), "%d", lo); | ||||||
| } | ||||||
| } | ||||||
| /* | ||||||
|
|
@@ -352,9 +352,9 @@ abbreviate_list_into_string(char *str, int max, int *list, int nlist) | |||||
| strcpy(&str[strlen(str)], ", "); | ||||||
| } | ||||||
| if (lo != hi) { | ||||||
| snprintf(&str[strlen(str)], max - strlen(str), "%d - %d", lo, hi); | ||||||
| } else { | ||||||
| snprintf(&str[strlen(str)], max - strlen(str), "%d", lo); | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
@@ -460,7 +460,7 @@ ompi_report_comm_methods(int called_from_location) | |||||
|
|
||||||
| len = strlen(opal_process_info.nodename) + 100; | ||||||
| hoststring = malloc(len + 1); | ||||||
| snprintf(hoststring, len + 1, "Host %d [%s] ranks ", | ||||||
| myleaderrank, opal_process_info.nodename); | ||||||
|
|
||||||
| abbreviate_list_into_string(&hoststring[strlen(hoststring)], | ||||||
|
|
@@ -642,7 +642,7 @@ ompi_report_comm_methods(int called_from_location) | |||||
| // 2: 2d table | ||||||
| if (nleaderranks <= max2Dprottable) { | ||||||
| char *str, *p; | ||||||
| int tmp, per, has_ucx_transport, bufsize; | ||||||
| int strlens[NUM_COMM_METHODS]; | ||||||
|
|
||||||
| // characters per entry in the 2d table, must be large enough | ||||||
|
|
@@ -668,11 +668,11 @@ ompi_report_comm_methods(int called_from_location) | |||||
| if (tmp+1 > per) { per = tmp+1; } | ||||||
| } | ||||||
| } | ||||||
| bufsize = nleaderranks * per + 1; | ||||||
| str = malloc(bufsize); | ||||||
| p = str; | ||||||
| for (i=0; i<nleaderranks; ++i) { | ||||||
| snprintf(p, bufsize - (p - str), "%d", i); | ||||||
| for (j=(int)strlen(p); j<per; ++j) { | ||||||
| p[j] = ' '; | ||||||
| } | ||||||
|
|
@@ -698,12 +698,12 @@ ompi_report_comm_methods(int called_from_location) | |||||
| for (k=0; k<nleaderranks; ++k) { | ||||||
| char *method_string; | ||||||
| char ucx_label[20]; | ||||||
|
|
||||||
| method_string = comm_method_to_string(method[i * nleaderranks + k]); | ||||||
| if (0 == strncmp(method_string, UCX_TAG, strlen(UCX_TAG))) { | ||||||
| n = lookup_string_in_conversion_struct(&comm_method_string_conversion, | ||||||
| method_string); | ||||||
| snprintf(ucx_label, sizeof(ucx_label), "ucx[%3d]", n); | ||||||
| strcat(p, ucx_label); | ||||||
| methods_used[n / 8] |= (1 << (n % 8)); | ||||||
| has_ucx_transport = 1; | ||||||
|
|
@@ -755,7 +755,7 @@ ompi_report_comm_methods(int called_from_location) | |||||
| } | ||||||
| else if (nleaderranks <= max2D1Cprottable) { | ||||||
| char *str, *p; | ||||||
| int tmp, per, done, bufsize; | ||||||
| char char_code[NUM_COMM_METHODS], next_char; | ||||||
| int method_count[NUM_COMM_METHODS]; | ||||||
|
|
||||||
|
|
@@ -798,12 +798,13 @@ ompi_report_comm_methods(int called_from_location) | |||||
| } | ||||||
| } | ||||||
|
|
||||||
| bufsize = nleaderranks * (per + 1) + 1; | ||||||
|
There was a problem hiding this comment. The buffer size calculation doesn't account for the initial header string '0 1 2 3 ' which is 8 characters. This could lead to a buffer overflow when writing to
Suggested change
There was a problem hiding this comment. This comment is incorrect. It accounts for the initial string by starting the loop below from 4 instead of 0. What I don't understand in this code is why it assumes there are always at least 4 leaderranks ? There was a problem hiding this comment. We're in a branch where we have a minimum number of leaderranks: https://github.com/open-mpi/ompi/pull/13468/files#diff-4bcdf923dce180dbcc719b3472dd10527a4c285021962e918f5365a75eac81f6L756 I think copilot is right that I messed up the size computation (technically, that was copilot and I didn't pay enough attention ^^) There was a problem hiding this comment. There are two usages of the allocated string. The first one requires |
||||||
| str = malloc(bufsize); | ||||||
| p = str; | ||||||
| snprintf(p, bufsize, "0 1 2 3 "); | ||||||
| p += 8; | ||||||
| for (i=4; i<nleaderranks; i+=4) { | ||||||
| snprintf(p, bufsize - (p - str), "%d", i); | ||||||
| for (j=(int)strlen(p); j<8; ++j) { | ||||||
|
There was a problem hiding this comment. Trying to understand the memory allocation above I got totally confused by this piece of code. Why is j looping up to 8 instead of up to |
||||||
| p[j] = ' '; | ||||||
| } | ||||||
|
|
@@ -974,13 +975,14 @@ ompi_report_comm_methods(int called_from_location) | |||||
| if (is_nonconformist) { | ||||||
| char *str = malloc(1024); | ||||||
devreal marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
| // int first = 1; | ||||||
| snprintf(str, 1024, " host %d:", i); | ||||||
| for (k=0; k<NUM_COMM_METHODS; ++k) { | ||||||
| if (method_count[k] > 0) { | ||||||
| // if (!first) { | ||||||
| // strcat(str, " /"); | ||||||
| // } | ||||||
| snprintf(&str[strlen(str)], | ||||||
| 1024 - strlen(str), | ||||||
| " [%dx %s]", | ||||||
| method_count[k], | ||||||
| comm_method_to_string(k)); | ||||||
|
|
||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.