Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #1824: Replaced most remaining sprintf with safer snprint #4003

Merged
merged 3 commits into from
Feb 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 10 additions & 9 deletions java/src/jni/h5util.c
Original file line number Diff line number Diff line change
Expand Up @@ -1192,7 +1192,8 @@ h5str_sprintf(JNIEnv *env, h5str_t *out_str, hid_t container, hid_t tid, void *i
* object.
*/

if (NULL == (this_str = (char *)malloc(64)))
const size_t size = 64;
if (NULL == (this_str = (char *)malloc(size)))
H5_JNI_FATAL_ERROR(ENVONLY, "h5str_sprintf: failed to allocate string buffer");

if ((obj = H5Rdereference2(container, H5P_DEFAULT, H5R_OBJECT, cptr)) < 0)
Expand All @@ -1206,25 +1207,25 @@ h5str_sprintf(JNIEnv *env, h5str_t *out_str, hid_t container, hid_t tid, void *i

switch (oi.type) {
case H5O_TYPE_GROUP:
if (sprintf(this_str, "%s %s", H5_TOOLS_GROUP, obj_tok_str) < 0)
H5_JNI_FATAL_ERROR(ENVONLY, "h5str_sprintf: sprintf failure");
if (snprintf(this_str, size, "%s %s", H5_TOOLS_GROUP, obj_tok_str) < 0)
H5_JNI_FATAL_ERROR(ENVONLY, "h5str_sprintf: snprintf failure");
break;

case H5O_TYPE_DATASET:
if (sprintf(this_str, "%s %s", H5_TOOLS_DATASET, obj_tok_str) < 0)
H5_JNI_FATAL_ERROR(ENVONLY, "h5str_sprintf: sprintf failure");
if (snprintf(this_str, size, "%s %s", H5_TOOLS_DATASET, obj_tok_str) < 0)
H5_JNI_FATAL_ERROR(ENVONLY, "h5str_sprintf: snprintf failure");
break;

case H5O_TYPE_NAMED_DATATYPE:
if (sprintf(this_str, "%s %s", H5_TOOLS_DATATYPE, obj_tok_str) < 0)
H5_JNI_FATAL_ERROR(ENVONLY, "h5str_sprintf: sprintf failure");
if (snprintf(this_str, size, "%s %s", H5_TOOLS_DATATYPE, obj_tok_str) < 0)
H5_JNI_FATAL_ERROR(ENVONLY, "h5str_sprintf: snprintf failure");
break;

case H5O_TYPE_UNKNOWN:
case H5O_TYPE_NTYPES:
default:
if (sprintf(this_str, "%u-%s", (unsigned)oi.type, obj_tok_str) < 0)
H5_JNI_FATAL_ERROR(ENVONLY, "h5str_sprintf: sprintf failure");
if (snprintf(this_str, size, "%u-%s", (unsigned)oi.type, obj_tok_str) < 0)
H5_JNI_FATAL_ERROR(ENVONLY, "h5str_sprintf: snprintf failure");
break;
}

Expand Down
33 changes: 17 additions & 16 deletions src/H5FDfamily.c
Original file line number Diff line number Diff line change
Expand Up @@ -234,27 +234,28 @@ H5FD__family_get_default_printf_filename(const char *old_filename)
HGOTO_ERROR(H5E_VFL, H5E_CANTALLOC, NULL, "can't allocate new filename buffer");

/* Determine if filename contains a ".h5" extension. */
if ((file_extension = strstr(old_filename, ".h5"))) {
file_extension = strstr(old_filename, ".h5");
if (file_extension) {
/* Insert the printf format between the filename and ".h5" extension. */
strcpy(tmp_buffer, old_filename);
file_extension = strstr(tmp_buffer, ".h5");
sprintf(file_extension, "%s%s", suffix, ".h5");
intptr_t beginningLength = file_extension - old_filename;
snprintf(tmp_buffer, new_filename_len, "%.*s%s%s", (int)beginningLength, old_filename, suffix, ".h5");
}
else if ((file_extension = strrchr(old_filename, '.'))) {
char *new_extension_loc = NULL;

else {
/* If the filename doesn't contain a ".h5" extension, but contains
* AN extension, just insert the printf format before that extension.
*/
strcpy(tmp_buffer, old_filename);
new_extension_loc = strrchr(tmp_buffer, '.');
sprintf(new_extension_loc, "%s%s", suffix, file_extension);
}
else {
/* If the filename doesn't contain an extension at all, just insert
* the printf format at the end of the filename.
*/
snprintf(tmp_buffer, new_filename_len, "%s%s", old_filename, suffix);
file_extension = strrchr(old_filename, '.');
if (file_extension) {
intptr_t beginningLength = file_extension - old_filename;
snprintf(tmp_buffer, new_filename_len, "%.*s%s%s", (int)beginningLength, old_filename, suffix,
file_extension);
}
else {
/* If the filename doesn't contain an extension at all, just insert
* the printf format at the end of the filename.
*/
snprintf(tmp_buffer, new_filename_len, "%s%s", old_filename, suffix);
}
}

ret_value = tmp_buffer;
Expand Down
33 changes: 17 additions & 16 deletions src/H5FDsplitter.c
Original file line number Diff line number Diff line change
Expand Up @@ -532,27 +532,28 @@ H5FD__splitter_get_default_wo_path(char *new_path, size_t new_path_len, const ch
HGOTO_ERROR(H5E_VFL, H5E_CANTSET, FAIL, "filename exceeds max length");

/* Determine if filename contains a ".h5" extension. */
if ((file_extension = strstr(base_filename, ".h5"))) {
file_extension = strstr(base_filename, ".h5");
if (file_extension) {
/* Insert the suffix between the filename and ".h5" extension. */
strcpy(new_path, base_filename);
file_extension = strstr(new_path, ".h5");
sprintf(file_extension, "%s%s", suffix, ".h5");
intptr_t beginningLength = file_extension - base_filename;
snprintf(new_path, new_path_len, "%.*s%s%s", (int)beginningLength, base_filename, suffix, ".h5");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah clever and always nicer to do things in a single snprintf call!

}
else if ((file_extension = strrchr(base_filename, '.'))) {
char *new_extension_loc = NULL;

else {
/* If the filename doesn't contain a ".h5" extension, but contains
* AN extension, just insert the suffix before that extension.
*/
strcpy(new_path, base_filename);
new_extension_loc = strrchr(new_path, '.');
sprintf(new_extension_loc, "%s%s", suffix, file_extension);
}
else {
/* If the filename doesn't contain an extension at all, just insert
* the suffix at the end of the filename.
*/
snprintf(new_path, new_path_len, "%s%s", base_filename, suffix);
file_extension = strrchr(base_filename, '.');
if (file_extension) {
intptr_t beginningLength = file_extension - base_filename;
snprintf(new_path, new_path_len, "%.*s%s%s", (int)beginningLength, base_filename, suffix,
file_extension);
}
else {
/* If the filename doesn't contain an extension at all, just insert
* the suffix at the end of the filename.
*/
snprintf(new_path, new_path_len, "%s%s", base_filename, suffix);
}
}

done:
Expand Down
22 changes: 11 additions & 11 deletions test/API/H5_api_async_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ test_multi_dataset_io(void)
/* Loop over datasets */
for (i = 0; i < 5; i++) {
/* Set dataset name */
sprintf(dset_name, "dset%d", i);
snprintf(dset_name, sizeof(dset_name), "dset%d", i);

/* Create the dataset asynchronously */
if ((dset_id[i] = H5Dcreate_async(file_id, dset_name, H5T_NATIVE_INT, space_id, H5P_DEFAULT,
Expand Down Expand Up @@ -450,7 +450,7 @@ test_multi_dataset_io(void)
/* Loop over datasets */
for (i = 0; i < 5; i++) {
/* Set dataset name */
sprintf(dset_name, "dset%d", i);
snprintf(dset_name, sizeof(dset_name), "dset%d", i);

/* Open the dataset asynchronously */
if ((dset_id[0] = H5Dopen_async(file_id, dset_name, H5P_DEFAULT, es_id)) < 0)
Expand Down Expand Up @@ -479,7 +479,7 @@ test_multi_dataset_io(void)
/* Loop over datasets */
for (i = 0; i < 5; i++) {
/* Set dataset name */
sprintf(dset_name, "dset%d", i);
snprintf(dset_name, sizeof(dset_name), "dset%d", i);

/* Open the dataset asynchronously */
if ((dset_id[0] = H5Dopen_async(file_id, dset_name, H5P_DEFAULT, es_id)) < 0)
Expand Down Expand Up @@ -619,7 +619,7 @@ test_multi_file_dataset_io(void)
/* Loop over files */
for (i = 0; i < 5; i++) {
/* Set file name */
sprintf(file_name, ASYNC_API_TEST_FILE_PRINTF, i);
snprintf(file_name, sizeof(file_name), ASYNC_API_TEST_FILE_PRINTF, i);

/* Create file asynchronously */
if ((file_id[i] =
Expand Down Expand Up @@ -761,7 +761,7 @@ test_multi_file_dataset_io(void)
/* Loop over files */
for (i = 0; i < 5; i++) {
/* Set file name */
sprintf(file_name, ASYNC_API_TEST_FILE_PRINTF, i);
snprintf(file_name, sizeof(file_name), ASYNC_API_TEST_FILE_PRINTF, i);

/* Open the file asynchronously */
if ((file_id[0] = H5Fopen_async(file_name, H5F_ACC_RDWR, H5P_DEFAULT, es_id)) < 0)
Expand Down Expand Up @@ -799,7 +799,7 @@ test_multi_file_dataset_io(void)
/* Loop over files */
for (i = 0; i < 5; i++) {
/* Set file name */
sprintf(file_name, ASYNC_API_TEST_FILE_PRINTF, i);
snprintf(file_name, sizeof(file_name), ASYNC_API_TEST_FILE_PRINTF, i);

/* Open the file asynchronously */
if ((file_id[0] = H5Fopen_async(file_name, H5F_ACC_RDONLY, H5P_DEFAULT, es_id)) < 0)
Expand Down Expand Up @@ -929,7 +929,7 @@ test_multi_file_grp_dset_io(void)
/* Loop over files */
for (i = 0; i < 5; i++) {
/* Set file name */
sprintf(file_name, ASYNC_API_TEST_FILE_PRINTF, i);
snprintf(file_name, sizeof(file_name), ASYNC_API_TEST_FILE_PRINTF, i);

/* Create file asynchronously */
if ((file_id = H5Fcreate_async(file_name, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT, es_id)) <
Expand Down Expand Up @@ -981,7 +981,7 @@ test_multi_file_grp_dset_io(void)
/* Loop over files */
for (i = 0; i < 5; i++) {
/* Set file name */
sprintf(file_name, ASYNC_API_TEST_FILE_PRINTF, i);
snprintf(file_name, sizeof(file_name), ASYNC_API_TEST_FILE_PRINTF, i);

/* Open the file asynchronously */
if ((file_id = H5Fopen_async(file_name, H5F_ACC_RDONLY, H5P_DEFAULT, es_id)) < 0)
Expand Down Expand Up @@ -1039,7 +1039,7 @@ test_multi_file_grp_dset_io(void)
/* Loop over files */
for (i = 0; i < 5; i++) {
/* Set file name */
sprintf(file_name, ASYNC_API_TEST_FILE_PRINTF, i);
snprintf(file_name, sizeof(file_name), ASYNC_API_TEST_FILE_PRINTF, i);

/* Create file asynchronously */
if ((file_id = H5Fcreate_async(file_name, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT, es_id)) <
Expand Down Expand Up @@ -1096,7 +1096,7 @@ test_multi_file_grp_dset_io(void)
/* Loop over files */
for (i = 0; i < 5; i++) {
/* Set file name */
sprintf(file_name, ASYNC_API_TEST_FILE_PRINTF, i);
snprintf(file_name, sizeof(file_name), ASYNC_API_TEST_FILE_PRINTF, i);

/* Open the file asynchronously */
if ((file_id = H5Fopen_async(file_name, H5F_ACC_RDONLY, H5P_DEFAULT, es_id)) < 0)
Expand Down Expand Up @@ -2676,7 +2676,7 @@ cleanup_files(void)

H5Fdelete(ASYNC_API_TEST_FILE, H5P_DEFAULT);
for (i = 0; i <= max_printf_file; i++) {
snprintf(file_name, 64, ASYNC_API_TEST_FILE_PRINTF, i);
snprintf(file_name, sizeof(file_name), ASYNC_API_TEST_FILE_PRINTF, i);
H5Fdelete(file_name, H5P_DEFAULT);
} /* end for */
}
Expand Down
2 changes: 1 addition & 1 deletion test/API/H5_api_attribute_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -10725,7 +10725,7 @@ test_attribute_many(void)

/* Create many attributes */
for (u = 0; u < ATTRIBUTE_MANY_NUMB; u++) {
sprintf(attrname, "many-%06u", u);
snprintf(attrname, sizeof(attrname), "many-%06u", u);

if ((attr_id = H5Acreate2(group_id, attrname, attr_dtype, space_id, H5P_DEFAULT, H5P_DEFAULT)) < 0) {
H5_FAILED();
Expand Down
17 changes: 10 additions & 7 deletions test/API/H5_api_dataset_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -1218,7 +1218,7 @@ test_create_dataset_random_shapes(void)
goto error;
}

sprintf(name, "%s%zu", DATASET_SHAPE_TEST_DSET_BASE_NAME, i + 1);
snprintf(name, sizeof(name), "%s%zu", DATASET_SHAPE_TEST_DSET_BASE_NAME, i + 1);

if ((dset_id = H5Dcreate2(group_id, name, dset_dtype, space_id, H5P_DEFAULT, H5P_DEFAULT,
H5P_DEFAULT)) < 0) {
Expand Down Expand Up @@ -1316,7 +1316,7 @@ test_create_dataset_predefined_types(void)
generate_random_dataspace(DATASET_PREDEFINED_TYPE_TEST_SPACE_RANK, NULL, NULL, false)) < 0)
TEST_ERROR;

sprintf(name, "%s%zu", DATASET_PREDEFINED_TYPE_TEST_BASE_NAME, i);
snprintf(name, sizeof(name), "%s%zu", DATASET_PREDEFINED_TYPE_TEST_BASE_NAME, i);

if ((dset_id = H5Dcreate2(group_id, name, predefined_type_test_table[i], fspace_id, H5P_DEFAULT,
H5P_DEFAULT, H5P_DEFAULT)) < 0) {
Expand Down Expand Up @@ -2154,7 +2154,8 @@ test_create_dataset_creation_properties(void)
PART_ERROR(DCPL_alloc_time_test);
}

sprintf(name, "%s%zu", DATASET_CREATION_PROPERTIES_TEST_ALLOC_TIMES_BASE_NAME, i);
snprintf(name, sizeof(name), "%s%zu", DATASET_CREATION_PROPERTIES_TEST_ALLOC_TIMES_BASE_NAME,
i);

if ((dset_id = H5Dcreate2(group_id, name, dset_dtype, fspace_id, H5P_DEFAULT, dcpl_id,
H5P_DEFAULT)) < 0) {
Expand Down Expand Up @@ -2230,7 +2231,8 @@ test_create_dataset_creation_properties(void)
PART_ERROR(DCPL_attr_crt_order_test);
}

sprintf(name, "%s%zu", DATASET_CREATION_PROPERTIES_TEST_CRT_ORDER_BASE_NAME, i);
snprintf(name, sizeof(name), "%s%zu", DATASET_CREATION_PROPERTIES_TEST_CRT_ORDER_BASE_NAME,
i);

if ((dset_id = H5Dcreate2(group_id, name, dset_dtype, fspace_id, H5P_DEFAULT, dcpl_id,
H5P_DEFAULT)) < 0) {
Expand Down Expand Up @@ -2363,7 +2365,8 @@ test_create_dataset_creation_properties(void)
PART_ERROR(DCPL_fill_time_property_test);
}

sprintf(name, "%s%zu", DATASET_CREATION_PROPERTIES_TEST_FILL_TIMES_BASE_NAME, i);
snprintf(name, sizeof(name), "%s%zu", DATASET_CREATION_PROPERTIES_TEST_FILL_TIMES_BASE_NAME,
i);

if ((dset_id = H5Dcreate2(group_id, name, dset_dtype, fspace_id, H5P_DEFAULT, dcpl_id,
H5P_DEFAULT)) < 0) {
Expand Down Expand Up @@ -2931,7 +2934,7 @@ test_create_dataset_creation_properties(void)
}
}

sprintf(name, "%s%zu", DATASET_CREATION_PROPERTIES_TEST_LAYOUTS_BASE_NAME, i);
snprintf(name, sizeof(name), "%s%zu", DATASET_CREATION_PROPERTIES_TEST_LAYOUTS_BASE_NAME, i);

if ((dset_id =
H5Dcreate2(group_id, name, (H5D_COMPACT == layouts[i]) ? compact_dtype : dset_dtype,
Expand Down Expand Up @@ -3192,7 +3195,7 @@ test_create_many_dataset(void)
printf("\n");
for (i = 0; i < DATASET_NUMB; i++) {
printf("\r %u/%u", i + 1, DATASET_NUMB);
sprintf(dset_name, "dset_%02u", i);
snprintf(dset_name, sizeof(dset_name), "dset_%02u", i);
data = i % 256;

if ((dset_id = H5Dcreate2(group_id, dset_name, H5T_NATIVE_UCHAR, dataspace_id, H5P_DEFAULT,
Expand Down
2 changes: 1 addition & 1 deletion test/API/H5_api_file_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -948,7 +948,7 @@ test_flush_file(void)
}

for (u = 0; u < 10; u++) {
sprintf(dset_name, "Dataset %u", u);
snprintf(dset_name, sizeof(dset_name), "Dataset %u", u);

if ((dset_id = H5Dcreate2(file_id, dset_name, H5T_STD_U32LE, dspace_id, H5P_DEFAULT, H5P_DEFAULT,
H5P_DEFAULT)) < 0) {
Expand Down
8 changes: 4 additions & 4 deletions test/API/H5_api_group_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ test_create_many_groups(void)
printf("\n");
for (i = 0; i < GROUP_NUMB_MANY; i++) {
printf("\r %u/%u", i + 1, GROUP_NUMB_MANY);
sprintf(group_name, "group %02u", i);
snprintf(group_name, sizeof(group_name), "group %02u", i);
if ((child_group_id =
H5Gcreate2(parent_group_id, group_name, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT)) < 0) {
H5_FAILED();
Expand Down Expand Up @@ -342,11 +342,11 @@ create_group_recursive(hid_t parent_gid, unsigned counter)

printf("\r %u/%u", counter, GROUP_DEPTH);
if (counter == 1)
sprintf(gname, "2nd_child_group");
snprintf(gname, sizeof(gname), "2nd_child_group");
else if (counter == 2)
sprintf(gname, "3rd_child_group");
snprintf(gname, sizeof(gname), "3rd_child_group");
else
sprintf(gname, "%dth_child_group", counter + 1);
snprintf(gname, sizeof(gname), "%dth_child_group", counter + 1);
if ((child_gid = H5Gcreate2(parent_gid, gname, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT)) < 0) {
H5_FAILED();
printf(" couldn't create group '%s'\n", gname);
Expand Down
10 changes: 2 additions & 8 deletions test/filter_plugin.c
Original file line number Diff line number Diff line change
Expand Up @@ -847,10 +847,7 @@ test_creating_groups_using_plugins(hid_t fid)

/* Create multiple groups under the top-level group */
for (i = 0; i < N_SUBGROUPS; i++) {
char *sp = subgroup_name;

sp += snprintf(subgroup_name, sizeof(subgroup_name), SUBGROUP_PREFIX);
sprintf(sp, "%d", i);
snprintf(subgroup_name, sizeof(subgroup_name), SUBGROUP_PREFIX "%d", i);

if ((sub_gid = H5Gcreate2(gid, subgroup_name, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT)) < 0)
TEST_ERROR;
Expand Down Expand Up @@ -906,10 +903,7 @@ test_opening_groups_using_plugins(hid_t fid)

/* Open all the sub-groups under the top-level group */
for (i = 0; i < N_SUBGROUPS; i++) {
char *sp = subgroup_name;

sp += snprintf(subgroup_name, sizeof(subgroup_name), SUBGROUP_PREFIX);
sprintf(sp, "%d", i);
snprintf(subgroup_name, sizeof(subgroup_name), SUBGROUP_PREFIX "%d", i);

if ((sub_gid = H5Gopen2(gid, subgroup_name, H5P_DEFAULT)) < 0)
TEST_ERROR;
Expand Down
Loading