From c7f2dfa0d715c3d2e43a5ff8ac65a8ab618df4d9 Mon Sep 17 00:00:00 2001 From: "Christopher M. Pierce" Date: Mon, 5 Jan 2026 02:03:20 -0500 Subject: [PATCH 01/29] add clang-tidy check --- .clang-tidy | 111 +++++++++++++++++++++++++++++++++++++++++++++++++ CMakeLists.txt | 19 +++++++++ 2 files changed, 130 insertions(+) create mode 100644 .clang-tidy diff --git a/.clang-tidy b/.clang-tidy new file mode 100644 index 0000000..a69a54e --- /dev/null +++ b/.clang-tidy @@ -0,0 +1,111 @@ +--- +Checks: > + -*, + bugprone-*, + -bugprone-easily-swappable-parameters, + -bugprone-exception-escape, + -bugprone-macro-parentheses, + cert-*, + google-*, + -google-readability-braces-around-statements, + -google-readability-todo, + misc-*, + -misc-const-correctness, + -misc-include-cleaner, + -misc-non-private-member-variables-in-classes, + -misc-unused-parameters, + -misc-use-anonymous-namespace, + modernize-*, + -modernize-avoid-c-arrays, + -modernize-return-braced-init-list, + -modernize-use-auto, + -modernize-use-default-member-init, + -modernize-use-equal-delete, + -modernize-use-nodiscard, + -modernize-use-nullptr, + -modernize-use-override, + -modernize-use-trailing-return-type, + performance-*, + portability-*, + readability-*, + -readability-braces-around-statements, + -readability-else-after-return, + -readability-function-cognitive-complexity, + -readability-identifier-length, + -readability-implicit-bool-conversion, + -readability-magic-numbers, + -readability-named-parameter, + -readability-uppercase-literal-suffix + +# Treat important categories as errors (customize as needed) +WarningsAsErrors: > + bugprone-*, + cert-*, + misc-*, + performance-* + +# Only apply checks to YOUR project headers (not third-party) +HeaderFilterRegex: 'parcel.h' + +# Don't apply automatic formatting fixes +FormatStyle: none + +# Configuration for specific checks +CheckOptions: + # Naming conventions + - key: readability-identifier-naming.VariableCase + value: lower_case + - key: readability-identifier-naming.FunctionCase + value: lower_case + - key: readability-identifier-naming.StructCase + value: lower_case + - key: readability-identifier-naming.EnumCase + value: CamelCase + - key: readability-identifier-naming.ConstantCase + value: UPPER_CASE + - key: readability-identifier-naming.MacroCase + value: UPPER_CASE + - key: readability-identifier-naming.MemberCase + value: lower_case + - key: readability-identifier-naming.MemberIgnoredRegexp + value: '^_.*' + + # Function length limits + - key: readability-function-size.LineThreshold + value: '500' + - key: readability-function-size.StatementThreshold + value: '200' + + # Performance tuning + - key: performance-move-const-arg.CheckTriviallyCopyableMove + value: 'false' + - key: performance-for-range-copy.WarnOnAllAutoCopies + value: 'true' + - key: performance-unnecessary-copy-initialization.AllowedTypes + value: '' + + # Enhanced const correctness for better optimization + - key: misc-const-correctness.AnalyzeValues + value: 'true' + - key: misc-const-correctness.AnalyzeReferences + value: 'true' + - key: misc-const-correctness.WarnPointersAsValues + value: 'false' + - key: misc-const-correctness.TransformValues + value: 'true' + - key: misc-const-correctness.TransformReferences + value: 'true' + + # Bounds checking for array operations + - key: cppcoreguidelines-pro-bounds-constant-array-index.GslHeader + value: '' + - key: cppcoreguidelines-pro-bounds-constant-array-index.IncludeStyle + value: 'llvm' + + # Modernize settings + - key: modernize-use-default-member-init.UseAssignment + value: 'true' + + # Misc settings + - key: misc-throw-by-value-catch-by-reference.CheckThrowTemporaries + value: 'true' diff --git a/CMakeLists.txt b/CMakeLists.txt index 07d6b68..419419a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -32,3 +32,22 @@ enable_testing() # Add tests subdirectory add_subdirectory(tests) + +# Setup clang-tidy for header-only library +find_program(CLANG_TIDY_EXE NAMES clang-tidy) +if(CLANG_TIDY_EXE) + add_custom_target(clang-tidy ALL + COMMAND ${CLANG_TIDY_EXE} + --config-file=${CMAKE_SOURCE_DIR}/.clang-tidy + ${CMAKE_SOURCE_DIR}/parcel.h + -- + -I${CMAKE_SOURCE_DIR} + -I${HDF5_INCLUDE_DIRS} + WORKING_DIRECTORY ${CMAKE_SOURCE_DIR} + COMMENT "Running clang-tidy on parcel.h" + VERBATIM + ) +else() + message(WARNING "clang-tidy not found") +endif() + From add99d4e7bb5eeda8807a5975022899ea620e7b2 Mon Sep 17 00:00:00 2001 From: "Christopher M. Pierce" Date: Mon, 5 Jan 2026 02:11:19 -0500 Subject: [PATCH 02/29] use default cmake for clang-tidy --- CMakeLists.txt | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 419419a..7ad6e85 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -26,28 +26,22 @@ add_library(unity STATIC ) target_include_directories(unity PUBLIC Unity-2.6.1) target_compile_definitions(unity PUBLIC UNITY_INCLUDE_DOUBLE) +# Disable clang-tidy for third-party code +set_target_properties(unity PROPERTIES CXX_CLANG_TIDY "") +set_target_properties(unity PROPERTIES C_CLANG_TIDY "") -# Enable testing -enable_testing() - -# Add tests subdirectory -add_subdirectory(tests) - -# Setup clang-tidy for header-only library +# Setup clang-tidy (must be before add_subdirectory) find_program(CLANG_TIDY_EXE NAMES clang-tidy) if(CLANG_TIDY_EXE) - add_custom_target(clang-tidy ALL - COMMAND ${CLANG_TIDY_EXE} - --config-file=${CMAKE_SOURCE_DIR}/.clang-tidy - ${CMAKE_SOURCE_DIR}/parcel.h - -- - -I${CMAKE_SOURCE_DIR} - -I${HDF5_INCLUDE_DIRS} - WORKING_DIRECTORY ${CMAKE_SOURCE_DIR} - COMMENT "Running clang-tidy on parcel.h" - VERBATIM - ) + set(CMAKE_CXX_CLANG_TIDY "${CLANG_TIDY_EXE}") + set(CMAKE_C_CLANG_TIDY "${CLANG_TIDY_EXE}") else() message(WARNING "clang-tidy not found") endif() +# Enable testing +enable_testing() + +# Add tests subdirectory +add_subdirectory(tests) + From 9d2e79116bb482e6514326a1bd519ef352b46cd9 Mon Sep 17 00:00:00 2001 From: "Christopher M. Pierce" Date: Mon, 5 Jan 2026 18:08:52 -0500 Subject: [PATCH 03/29] dealing with false positive clang-tidy --- .clang-tidy | 3 ++- tests/utils.h | 10 +++++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index a69a54e..ca05210 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -5,6 +5,7 @@ Checks: > -bugprone-easily-swappable-parameters, -bugprone-exception-escape, -bugprone-macro-parentheses, + -bugprone-multi-level-implicit-pointer-conversion, cert-*, google-*, -google-readability-braces-around-statements, @@ -74,7 +75,7 @@ CheckOptions: - key: readability-function-size.LineThreshold value: '500' - key: readability-function-size.StatementThreshold - value: '200' + value: '300' # Performance tuning - key: performance-move-const-arg.CheckTriviallyCopyableMove diff --git a/tests/utils.h b/tests/utils.h index e6640d7..3d5539b 100644 --- a/tests/utils.h +++ b/tests/utils.h @@ -22,11 +22,15 @@ double _atol = (atol); \ double _diff = fabs(_actual - _expected); \ double _tolerance = _atol + _rtol * fabs(_expected); \ + int _err; \ if (_diff > _tolerance) { \ char _msg[256]; \ - snprintf(_msg, sizeof(_msg), \ - "Expected %.15g, was %.15g (diff=%.3e, tol=%.3e)", \ - _expected, _actual, _diff, _tolerance); \ + _err = snprintf(_msg, sizeof(_msg), \ + "Expected %.15g, was %.15g (diff=%.3e, tol=%.3e)", \ + _expected, _actual, _diff, _tolerance); \ + if (_err < 0) { \ + UNITY_TEST_FAIL(__LINE__, "snprintf failed in error message"); \ + } \ UNITY_TEST_FAIL(__LINE__, _msg); \ } \ } while(0) From 0735967d71276271231526fa2c1457f75f0e2b12 Mon Sep 17 00:00:00 2001 From: "Christopher M. Pierce" Date: Mon, 5 Jan 2026 18:17:05 -0500 Subject: [PATCH 04/29] setting up script for valgrind testing --- leak-check.sh | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 leak-check.sh diff --git a/leak-check.sh b/leak-check.sh new file mode 100644 index 0000000..c96eada --- /dev/null +++ b/leak-check.sh @@ -0,0 +1,3 @@ +# valgrind --leak-check=full --show-leak-kinds=all ./build/tests/test_read +valgrind --leak-check=full --show-leak-kinds=all ./build/tests/test_write +# valgrind --leak-check=full --show-leak-kinds=all ./build/tests/test_read_write \ No newline at end of file From dc2e590d61b2288e242c6508312fd12200f51e31 Mon Sep 17 00:00:00 2001 From: "Christopher M. Pierce" Date: Mon, 5 Jan 2026 18:21:02 -0500 Subject: [PATCH 05/29] make clang-tidy optional --- CMakeLists.txt | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 7ad6e85..39c62e1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -31,12 +31,15 @@ set_target_properties(unity PROPERTIES CXX_CLANG_TIDY "") set_target_properties(unity PROPERTIES C_CLANG_TIDY "") # Setup clang-tidy (must be before add_subdirectory) -find_program(CLANG_TIDY_EXE NAMES clang-tidy) -if(CLANG_TIDY_EXE) - set(CMAKE_CXX_CLANG_TIDY "${CLANG_TIDY_EXE}") - set(CMAKE_C_CLANG_TIDY "${CLANG_TIDY_EXE}") -else() - message(WARNING "clang-tidy not found") +option(CLANG_TIDY "Run clang-tidy everytime the program is built" Off) +if(CLANG_TIDY) + find_program(CLANG_TIDY_EXE NAMES clang-tidy) + if(CLANG_TIDY_EXE) + set(CMAKE_CXX_CLANG_TIDY "${CLANG_TIDY_EXE}") + set(CMAKE_C_CLANG_TIDY "${CLANG_TIDY_EXE}") + else() + message(WARNING "clang-tidy not found") + endif() endif() # Enable testing From aad4f574e0b029e22cace338318211c3456c6d03 Mon Sep 17 00:00:00 2001 From: "Christopher M. Pierce" Date: Mon, 5 Jan 2026 20:06:14 -0500 Subject: [PATCH 06/29] add debug flags --- CMakeLists.txt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 39c62e1..ba76591 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -42,6 +42,11 @@ if(CLANG_TIDY) endif() endif() +option(DEBUG "Enable debug flags" Off) +if(DEBUG) + add_compile_options(-g) +endif() + # Enable testing enable_testing() From 23574ea511057741e60cb01c01ecb4d31f592518 Mon Sep 17 00:00:00 2001 From: "Christopher M. Pierce" Date: Mon, 5 Jan 2026 20:19:15 -0500 Subject: [PATCH 07/29] starting on clang-tidy issues --- parcel.h | 76 +++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 50 insertions(+), 26 deletions(-) diff --git a/parcel.h b/parcel.h index 76160c2..1e946f8 100644 --- a/parcel.h +++ b/parcel.h @@ -899,7 +899,8 @@ static int attribute_exists(hid_t loc_id, const char *attr_name) { * Returns PMD_SUCCESS on success, error code on failure */ static pmd_status read_string_attribute(hid_t loc_id, const char *attr_name, char **value_out) { - hid_t attr_id, atype_id; + hid_t attr_id; + hid_t atype_id; char *str_value = NULL; htri_t is_variable; pmd_status status; @@ -1103,7 +1104,7 @@ static pmd_status create_parent_directory(const char *filepath) { */ static int parent_directory_exists(const char *filepath) { char *path_copy; - char *first_T; + char *first_format_char; char *last_sep; int exists = 0; @@ -1119,10 +1120,10 @@ static int parent_directory_exists(const char *filepath) { } /* If pattern contains %T, find the parent directory before the first %T */ - first_T = strstr(path_copy, "%T"); - if (first_T) { + first_format_char = strstr(path_copy, "%T"); + if (first_format_char) { /* Find last separator before %T */ - last_sep = first_T; + last_sep = first_format_char; while (last_sep > path_copy && *last_sep != '/' && *last_sep != '\\') { last_sep--; } @@ -1175,7 +1176,9 @@ static int parent_directory_exists(const char *filepath) { } static pmd_status write_string_attribute(hid_t loc_id, const char *attr_name, const char *value) { - hid_t aspace_id, atype_id, attr_id; + hid_t aspace_id; + hid_t atype_id; + hid_t attr_id; herr_t status; /* Create scalar dataspace */ @@ -1336,7 +1339,8 @@ static pmd_status write_root_attributes(hid_t file_id, pmd_series *series) { } static pmd_status write_double_attribute(hid_t loc_id, const char *attr_name, double value) { - hid_t aspace_id, attr_id; + hid_t aspace_id; + hid_t attr_id; herr_t status; /* Create scalar dataspace */ @@ -1378,7 +1382,8 @@ static pmd_status write_double_attribute(hid_t loc_id, const char *attr_name, do * @return PMD_SUCCESS or error code */ static pmd_status write_unit_dimension_attribute(hid_t loc_id, const pmd_unit_dimension *unit_dim) { - hid_t aspace_id, attr_id; + hid_t aspace_id; + hid_t attr_id; hsize_t dims[1] = {7}; double values[7]; herr_t status; @@ -1835,7 +1840,7 @@ pmd_status pmd_open_series(const char *filename, pmd_series **series_out, pmd_ac } /* Set actual_filename to NULL since we deleted the files */ - free((char*)actual_filename); + free(actual_filename); actual_filename = NULL; /* Set up series for FILE_BASED mode (files will be created on demand) */ @@ -2340,8 +2345,8 @@ static pmd_status parse_iteration_pattern(const char *pattern, iteration_pattern info->full_pattern = pattern; /* Find first %T */ - const char *first_T = strstr(pattern, "%T"); - if (!first_T) { + const char *first_format_char = strstr(pattern, "%T"); + if (!first_format_char) { /* No %T - treat whole thing as parent, empty first_segment */ info->scan_parent = strdup(pattern); info->first_segment = strdup(""); @@ -2355,7 +2360,7 @@ static pmd_status parse_iteration_pattern(const char *pattern, iteration_pattern /* Find last path separator before first %T to get scan parent * Check for both / and \ to support both HDF5 paths and Windows file paths */ - const char *last_slash = first_T; + const char *last_slash = first_format_char; while (last_slash > pattern && *last_slash != '/' && *last_slash != '\\') { last_slash--; } @@ -2424,7 +2429,7 @@ static pmd_status extract_iteration_from_name(const char *name, const char *patt const char *p = pattern; const char *n = name; char matched_number[64] = {0}; - int found_T = 0; + int found_format_char = 0; if (!name || !pattern || !iteration_out) { return PMD_ERROR_NULL_POINTER; @@ -2458,14 +2463,14 @@ static pmd_status extract_iteration_from_name(const char *name, const char *patt } size_t digit_len = n - digit_start; - if (!found_T) { + if (!found_format_char) { /* First %T - store matched digits */ if (digit_len >= sizeof(matched_number)) { return PMD_ERROR; } strncpy(matched_number, digit_start, digit_len); matched_number[digit_len] = '\0'; - found_T = 1; + found_format_char = 1; } else { /* Subsequent %T must match same digits */ if (strlen(matched_number) != digit_len || @@ -2488,7 +2493,7 @@ static pmd_status extract_iteration_from_name(const char *name, const char *patt return PMD_ERROR; } - if (!found_T) { + if (!found_format_char) { return PMD_ERROR; } @@ -2523,7 +2528,7 @@ static char* replace_iteration(const char *pattern, int64_t iteration) { } /* Count %T occurrences */ - int count = 0; + unsigned long count = 0; const char *p = pattern; while ((p = strstr(p, "%T")) != NULL) { count++; @@ -2537,7 +2542,11 @@ static char* replace_iteration(const char *pattern, int64_t iteration) { /* Format iteration number */ char iter_str[32]; - snprintf(iter_str, sizeof(iter_str), "%lld", (long long)iteration); + int status = snprintf(iter_str, sizeof(iter_str), "%lld", (long long)iteration); + if (status < 0) { + pmd_log(PMD_LOG_ERROR, "replace_iteration - Failed to format iteration count into string."); + return NULL; + } size_t iter_len = strlen(iter_str); /* Calculate result length: original - (count * 2) + (count * iter_len) */ @@ -2828,7 +2837,12 @@ pmd_status pmd_open_iteration(pmd_series *series, int64_t index, pmd_iteration * goto cleanup; } char full_path[PMD_PATH_MAX]; - snprintf(full_path, sizeof(full_path), "%s" PMD_PATH_SEP "%s", series->directory, filename); + int s_status = snprintf(full_path, sizeof(full_path), "%s" PMD_PATH_SEP "%s", series->directory, filename); + if (s_status < 0) { + status = PMD_ERROR; + pmd_log(PMD_LOG_ERROR, "pmd_open_iteration - failed to construct full path"); + goto cleanup; + } free(filename); /* Try to open existing file */ @@ -4020,7 +4034,8 @@ pmd_status pmd_free_particle_group(particle_group *pg) { } static pmd_status write_int64_attribute(hid_t loc_id, const char *attr_name, int64_t value) { - hid_t aspace_id, attr_id; + hid_t aspace_id; + hid_t attr_id; herr_t status; aspace_id = H5Screate(H5S_SCALAR); @@ -4053,7 +4068,8 @@ static pmd_status write_int64_attribute(hid_t loc_id, const char *attr_name, int */ static pmd_status _write_double_dataset(hid_t group_id, const char *name, const double *data, int64_t num_particles) { - hid_t dspace_id, dset_id; + hid_t dspace_id; + hid_t dset_id; hsize_t dims[1] = {(hsize_t)num_particles}; dspace_id = H5Screate_simple(1, dims, NULL); @@ -4195,7 +4211,8 @@ static pmd_status write_vector_record(hid_t parent_group_id, const char *record_ static pmd_status write_int64_dataset(hid_t group_id, const char *name, const int64_t *data, int64_t num_particles, const pmd_unit_dimension *unit_dim, double time_offset) { - hid_t dspace_id, dset_id; + hid_t dspace_id; + hid_t dset_id; hsize_t dims[1] = {(hsize_t)num_particles}; pmd_status status; @@ -4248,12 +4265,17 @@ static pmd_status write_int64_dataset(hid_t group_id, const char *name, const in pmd_status pmd_write_particle_group(pmd_iteration *iter, const particle_group *pg) { pmd_status status = PMD_SUCCESS; - hid_t particles_group_id = -1, species_group_id = -1; + hid_t particles_group_id = -1; + hid_t species_group_id = -1; char *particles_path = NULL; const double *position_components[3]; const double *momentum_components[3]; - double *offset_x, *offset_y, *offset_z; - int allocated_offset_x = 0, allocated_offset_y = 0, allocated_offset_z = 0; + double *offset_x; + double *offset_y; + double *offset_z; + int allocated_offset_x = 0; + int allocated_offset_y = 0; + int allocated_offset_z = 0; /* Unit dimensions for records */ pmd_unit_dimension position_dim = {1.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0}; /* length */ @@ -4506,7 +4528,9 @@ static pmd_status read_unit_si(hid_t loc_id, double *unit_si_out) { static pmd_status read_record_generic(hid_t group_id, const char *name, void *array, hid_t h5_type, size_t elem_size, int64_t num_particles, double *unit_si_out) { - hid_t dataset_id, group_id_local, attr_id; + hid_t dataset_id; + hid_t group_id_local; + hid_t attr_id; double unit_si = 1.0; if (!array) return PMD_ERROR_NULL_POINTER; From 393b03f2e0665155e7e2c0d89734dc6cf08f2827 Mon Sep 17 00:00:00 2001 From: "Christopher M. Pierce" Date: Mon, 5 Jan 2026 20:54:06 -0500 Subject: [PATCH 08/29] resolve clang-tidy warnings --- .clang-tidy | 7 ++ parcel.h | 88 +++++++++++++----- tests/test_cpp_api.cpp | 16 ++-- tests/test_read_write.c | 38 ++++---- tests/test_write.c | 198 ++++++++++++++++++++++------------------ 5 files changed, 208 insertions(+), 139 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index ca05210..6d65c15 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -7,15 +7,20 @@ Checks: > -bugprone-macro-parentheses, -bugprone-multi-level-implicit-pointer-conversion, cert-*, + -cert-err34-c, + -cert-dcl50-cpp, google-*, -google-readability-braces-around-statements, -google-readability-todo, + -google-readability-casting, misc-*, -misc-const-correctness, -misc-include-cleaner, -misc-non-private-member-variables-in-classes, -misc-unused-parameters, -misc-use-anonymous-namespace, + -misc-no-recursion, + -misc-definitions-in-headers, modernize-*, -modernize-avoid-c-arrays, -modernize-return-braced-init-list, @@ -26,7 +31,9 @@ Checks: > -modernize-use-nullptr, -modernize-use-override, -modernize-use-trailing-return-type, + -modernize-use-using, performance-*, + -performance-enum-size, portability-*, readability-*, -readability-braces-around-statements, diff --git a/parcel.h b/parcel.h index 1e946f8..b7bf585 100644 --- a/parcel.h +++ b/parcel.h @@ -673,8 +673,17 @@ static void pmd_log(pmd_log_level level, const char *format, ...) { va_list args; va_start(args, format); - fprintf(stderr, "%s: ", level_str); - vfprintf(stderr, format, args); + // Construct + int fstatus = fprintf(stderr, "%s: ", level_str); + if ((fstatus < 0) && (pmd_log_threshold > PMD_LOG_NONE)) { + printf("pmd_log - failed to construct log level message."); + } + + // Perform user formatting + int vstatus = vfprintf(stderr, format, args); + if ((vstatus < 0) && (pmd_log_threshold > PMD_LOG_NONE)) { + printf("pmd_log - failed to apply user formatting to log message."); + } va_end(args); } @@ -700,7 +709,7 @@ static pmd_status write_root_attributes(hid_t file_id, pmd_series *series); static pmd_status write_iteration_attributes(hid_t group_id); static pmd_status ensure_parent_groups(hid_t file_id, const char *path); static pmd_status write_series_root_attributes(pmd_series *series); -static pmd_status write_double_dataset(hid_t group_id, const char *name, const double *data, +static pmd_status write_double_record(hid_t group_id, const char *name, const double *data, int64_t num_particles, double unit_si, const pmd_unit_dimension *unit_dim, double time_offset); static pmd_status write_int64_dataset(hid_t group_id, const char *name, const int64_t *data, @@ -1330,7 +1339,11 @@ static pmd_status write_root_attributes(hid_t file_id, pmd_series *series) { time_t now = time(NULL); struct tm *tm_info = gmtime(&now); char date_str[64]; - strftime(date_str, sizeof(date_str), "%Y-%m-%d %H:%M:%S +0000", tm_info); + size_t sstatus = strftime(date_str, sizeof(date_str), "%Y-%m-%d %H:%M:%S +0000", tm_info); + if (sstatus < 0) { + pmd_log(PMD_LOG_ERROR, "write_root_attributes - failed to construct time string."); + return PMD_ERROR; + } status = write_string_attribute(file_id, "date", date_str); if (status != PMD_SUCCESS) return status; } @@ -1477,7 +1490,11 @@ static pmd_status ensure_parent_groups(hid_t file_id, const char *path) { /* Temporarily null-terminate at slash to get parent path */ *slash = '\0'; char parent_path[512]; - snprintf(parent_path, sizeof(parent_path), "/%s", path_copy); + int sstatus = snprintf(parent_path, sizeof(parent_path), "/%s", path_copy); + if (sstatus < 0) { + pmd_log(PMD_LOG_ERROR, "ensure_parent_groups - failed to extract parent path."); + return PMD_ERROR; + } *slash = '/'; /* Restore slash */ /* Try to open parent group, create if doesn't exist */ @@ -1520,7 +1537,9 @@ static pmd_status read_series_metadata_from_file(hid_t file_id, pmd_series *seri } /* Parse version (format: "X.Y.Z") for validation */ - int major, minor, revision; + int major; + int minor; + int revision; if (sscanf(openpmd_version, "%d.%d.%d", &major, &minor, &revision) != 3) { pmd_log(PMD_LOG_ERROR, "Invalid OpenPMD version format '%s' in '%s' (expected X.Y.Z)\n", openpmd_version, filename); @@ -1830,10 +1849,23 @@ pmd_status pmd_open_series(const char *filename, pmd_series **series_out, pmd_ac while ((del_entry = pmd_readdir(del_dir)) != NULL) { int64_t iter_index; if (extract_iteration_from_name(del_entry->d_name, pattern_info.first_segment, &iter_index) == PMD_SUCCESS) { + // Construct path to delete char del_path[PMD_PATH_MAX]; - snprintf(del_path, sizeof(del_path), "%s" PMD_PATH_SEP "%s", - pattern_info.scan_parent, del_entry->d_name); - remove(del_path); /* Delete the file */ + int sstatus = snprintf(del_path, sizeof(del_path), "%s" PMD_PATH_SEP "%s", + pattern_info.scan_parent, del_entry->d_name); + if (sstatus < 0) { + pmd_log(PMD_LOG_ERROR, "pmd_open_series - failed to construct path of file to delete in truncate mode."); + status = PMD_ERROR; + goto cleanup; + } + + // Delete the file + int rstatus = remove(del_path); + if (rstatus < 0) { + pmd_log(PMD_LOG_ERROR, "pmd_open_series - failed to delete existing iterations in truncate mode."); + status = PMD_ERROR; + goto cleanup; + } } } pmd_closedir(del_dir); @@ -1933,7 +1965,11 @@ pmd_status pmd_open_series(const char *filename, pmd_series **series_out, pmd_ac FILE *test = fopen(filename, "rb"); if (test) { file_exists = 1; - fclose(test); + int fstatus = fclose(test); + if (fstatus < 0) { + status = PMD_ERROR; + goto cleanup; + } } /* Opening existing file (PMD_RDONLY or PMD_RDWR) */ @@ -2223,6 +2259,8 @@ pmd_status pmd_get_iterations(pmd_series *series, int64_t **iterations, int *cou /* Validate full path if pattern has additional path components */ if (pattern_info.full_pattern && (strchr(pattern_info.full_pattern, '/') || strchr(pattern_info.full_pattern, '\\'))) { + int status; + /* Build full file path with iteration substituted */ char *rel_path = replace_iteration(series->iteration_format, iteration); if (!rel_path) { @@ -2231,7 +2269,11 @@ pmd_status pmd_get_iterations(pmd_series *series, int64_t **iterations, int *cou } char full_path[PMD_PATH_MAX]; - snprintf(full_path, sizeof(full_path), "%s" PMD_PATH_SEP "%s", series->directory, rel_path); + status = snprintf(full_path, sizeof(full_path), "%s" PMD_PATH_SEP "%s", series->directory, rel_path); + if (status < 0) { + pmd_log(PMD_LOG_ERROR, "pmd_get_iterations - failed to construct full path."); + return PMD_ERROR; + } free(rel_path); /* Check if file exists using fopen (standard C) */ @@ -2239,7 +2281,11 @@ pmd_status pmd_get_iterations(pmd_series *series, int64_t **iterations, int *cou if (!test_file) { continue; /* Path doesn't exist, skip */ } - fclose(test_file); + status = fclose(test_file); + if (status < 0){ + pmd_log(PMD_LOG_ERROR, "pmd_get_iterations - failed to close test file"); + return PMD_ERROR; + } } /* Grow array if needed */ @@ -2528,7 +2574,7 @@ static char* replace_iteration(const char *pattern, int64_t iteration) { } /* Count %T occurrences */ - unsigned long count = 0; + uint64_t count = 0; const char *p = pattern; while ((p = strstr(p, "%T")) != NULL) { count++; @@ -2727,7 +2773,7 @@ pmd_status pmd_open_iteration(pmd_series *series, int64_t index, pmd_iteration * /* Check for negative iteration index */ if (index < 0) { - pmd_log(PMD_LOG_ERROR, "Iteration index must be non-negative, got %lld", (long long)index); + pmd_log(PMD_LOG_ERROR, "Iteration index must be non-negative, got %lld", index); return PMD_ERROR; } @@ -4066,7 +4112,7 @@ static pmd_status write_int64_attribute(hid_t loc_id, const char *attr_name, int /** * Write double dataset with no attributes (private helper) */ -static pmd_status _write_double_dataset(hid_t group_id, const char *name, const double *data, +static pmd_status write_double_dataset(hid_t group_id, const char *name, const double *data, int64_t num_particles) { hid_t dspace_id; hid_t dset_id; @@ -4096,14 +4142,14 @@ static pmd_status _write_double_dataset(hid_t group_id, const char *name, const /** * Write double dataset with unitSI attribute, and optionally unitDimension and timeOffset */ -static pmd_status write_double_dataset(hid_t group_id, const char *name, const double *data, +static pmd_status write_double_record(hid_t group_id, const char *name, const double *data, int64_t num_particles, double unit_si, const pmd_unit_dimension *unit_dim, double time_offset) { pmd_status status; hid_t dset_id; /* Write the dataset */ - status = _write_double_dataset(group_id, name, data, num_particles); + status = write_double_dataset(group_id, name, data, num_particles); if (status != PMD_SUCCESS) return status; /* Reopen dataset to add attributes */ @@ -4181,7 +4227,7 @@ static pmd_status write_vector_record(hid_t parent_group_id, const char *record_ for (int i = 0; i < 3; i++) { if (component_data[i]) { /* Write dataset */ - status = _write_double_dataset(record_group_id, component_names[i], + status = write_double_dataset(record_group_id, component_names[i], component_data[i], num_particles); if (status != PMD_SUCCESS) { H5Gclose(record_group_id); @@ -4405,13 +4451,13 @@ pmd_status pmd_write_particle_group(pmd_iteration *iter, const particle_group *p /* Write time as scalar record if present (at species level) */ if (pg->t) { - status = write_double_dataset(species_group_id, "time", pg->t, pg->num_particles, 1.0, &time_dim, 0.0); + status = write_double_record(species_group_id, "time", pg->t, pg->num_particles, 1.0, &time_dim, 0.0); if (status != PMD_SUCCESS) goto cleanup; } /* Write optional scalar records (dimensionless) */ if (pg->weight) { - status = write_double_dataset(species_group_id, "weight", pg->weight, pg->num_particles, 1.0, &dimensionless, 0.0); + status = write_double_record(species_group_id, "weight", pg->weight, pg->num_particles, 1.0, &dimensionless, 0.0); if (status != PMD_SUCCESS) goto cleanup; } if (pg->status) { @@ -4561,7 +4607,7 @@ static pmd_status read_record_generic(hid_t group_id, const char *name, H5Sclose(dataspace_id); H5Dclose(dataset_id); pmd_log(PMD_LOG_ERROR, "Dataset '%s' has size %llu, expected %lld\n", - name, (unsigned long long)dims[0], (long long)num_particles); + name, dims[0], num_particles); return PMD_ERROR_FILE_FORMAT; } diff --git a/tests/test_cpp_api.cpp b/tests/test_cpp_api.cpp index e8d0067..0e0f44b 100644 --- a/tests/test_cpp_api.cpp +++ b/tests/test_cpp_api.cpp @@ -12,7 +12,7 @@ extern "C" { #include "../parcel.h" /* Custom C++ particle group using std::vector for storage */ -struct CppParticleGroup { +struct cpp_particle_group { int64_t num_particles; std::string species_type; @@ -38,7 +38,7 @@ struct CppParticleGroup { std::vector id; /* Constructor - allocate all arrays */ - CppParticleGroup(int64_t count) : num_particles(count) { + explicit cpp_particle_group(int64_t count) : num_particles(count) { x.resize(count); y.resize(count); z.resize(count); @@ -79,16 +79,16 @@ struct CppParticleGroup { } }; -void setUp(void) { +void setUp() { /* This is run before each test */ } -void tearDown(void) { +void tearDown() { /* This is run after each test */ } /* Test basic C++ API usage with std::vector-backed storage */ -void test_cpp_vector_backed_particle_group(void) { +void test_cpp_vector_backed_particle_group() { pmd_series *series; pmd_iteration *iter; pmd_status result; @@ -117,7 +117,7 @@ void test_cpp_vector_backed_particle_group(void) { TEST_ASSERT_TRUE(num_particles > 0); /* Create C++ particle group with std::vector storage */ - CppParticleGroup cpp_pg(num_particles); + cpp_particle_group cpp_pg(num_particles); cpp_pg.species_type = "electron"; /* Convert to C struct for reading */ @@ -151,7 +151,7 @@ void test_cpp_vector_backed_particle_group(void) { } /* Test selective reading with some NULL pointers */ -void test_cpp_selective_reading(void) { +void test_cpp_selective_reading() { pmd_series *series; pmd_iteration *iter; pmd_status result; @@ -222,7 +222,7 @@ void test_cpp_selective_reading(void) { } /* Main test runner */ -int main(void) { +int main() { // Turn off logging for tests pmd_set_log_level(PMD_LOG_NONE); diff --git a/tests/test_read_write.c b/tests/test_read_write.c index 482f35c..afa42a6 100644 --- a/tests/test_read_write.c +++ b/tests/test_read_write.c @@ -59,7 +59,7 @@ static void remove_directory(const char *path) { if (dir) { while ((entry = readdir(dir)) != NULL) { if (strcmp(entry->d_name, ".") != 0 && strcmp(entry->d_name, "..") != 0) { - snprintf(file_path, sizeof(file_path), "%s/%s", path, entry->d_name); + (void)snprintf(file_path, sizeof(file_path), "%s/%s", path, entry->d_name); struct stat st; if (stat(file_path, &st) == 0) { if (S_ISDIR(st.st_mode)) { @@ -99,24 +99,24 @@ static void test_write_and_read_particle_group_helper(const char *filename) { pmd_status result; particle_group write_pg; particle_group *read_pg = NULL; - const int64_t num_particles = 10000; - const int64_t num_iterations = 3; + const int64_t NUM_PARTICLES = 10000; + const int64_t NUM_ITERATIONS = 3; /* Allocate write arrays */ - double *write_x = (double*)malloc(num_particles * sizeof(double)); - double *write_y = (double*)malloc(num_particles * sizeof(double)); - double *write_z = (double*)malloc(num_particles * sizeof(double)); - double *write_t = (double*)malloc(num_particles * sizeof(double)); - double *write_px = (double*)malloc(num_particles * sizeof(double)); - double *write_py = (double*)malloc(num_particles * sizeof(double)); - double *write_pz = (double*)malloc(num_particles * sizeof(double)); - double *write_weight = (double*)malloc(num_particles * sizeof(double)); - int64_t *write_status = (int64_t*)malloc(num_particles * sizeof(int64_t)); - int64_t *write_id = (int64_t*)malloc(num_particles * sizeof(int64_t)); + double *write_x = (double*)malloc(NUM_PARTICLES * sizeof(double)); + double *write_y = (double*)malloc(NUM_PARTICLES * sizeof(double)); + double *write_z = (double*)malloc(NUM_PARTICLES * sizeof(double)); + double *write_t = (double*)malloc(NUM_PARTICLES * sizeof(double)); + double *write_px = (double*)malloc(NUM_PARTICLES * sizeof(double)); + double *write_py = (double*)malloc(NUM_PARTICLES * sizeof(double)); + double *write_pz = (double*)malloc(NUM_PARTICLES * sizeof(double)); + double *write_weight = (double*)malloc(NUM_PARTICLES * sizeof(double)); + int64_t *write_status = (int64_t*)malloc(NUM_PARTICLES * sizeof(int64_t)); + int64_t *write_id = (int64_t*)malloc(NUM_PARTICLES * sizeof(int64_t)); /* Setup write particle group */ memset(&write_pg, 0, sizeof(particle_group)); - write_pg.num_particles = num_particles; + write_pg.num_particles = NUM_PARTICLES; write_pg.species_type = "electron"; write_pg.x = write_x; write_pg.y = write_y; @@ -133,11 +133,11 @@ static void test_write_and_read_particle_group_helper(const char *filename) { result = pmd_open_series(filename, &series, PMD_TRUNC); TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, result); - for (int64_t iter_loop = 0; iter_loop < num_iterations; iter_loop++) { + for (int64_t iter_loop = 0; iter_loop < NUM_ITERATIONS; iter_loop++) { int64_t iter_idx = iter_loop * 5; /* Non-consecutive: 0, 5, 10 */ /* Initialize with iteration-dependent test values */ - for (int64_t i = 0; i < num_particles; i++) { + for (int64_t i = 0; i < NUM_PARTICLES; i++) { /* Position in meters - add mm per iteration */ write_x[i] = 0.1 + (double)i * 0.01 + (double)iter_idx * 0.001; write_y[i] = 0.2 + (double)i * 0.02 + (double)iter_idx * 0.002; @@ -176,7 +176,7 @@ static void test_write_and_read_particle_group_helper(const char *filename) { result = pmd_open_series(filename, &series, PMD_RDONLY); TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, result); - for (int64_t iter_loop = 0; iter_loop < num_iterations; iter_loop++) { + for (int64_t iter_loop = 0; iter_loop < NUM_ITERATIONS; iter_loop++) { int64_t iter_idx = iter_loop * 5; /* Non-consecutive: 0, 5, 10 */ result = pmd_open_iteration(series, iter_idx, &iter); @@ -190,9 +190,9 @@ static void test_write_and_read_particle_group_helper(const char *filename) { TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, result); /* Verify data matches expected values for this iteration */ - TEST_ASSERT_EQUAL_INT64(num_particles, read_pg->num_particles); + TEST_ASSERT_EQUAL_INT64(NUM_PARTICLES, read_pg->num_particles); - for (int64_t i = 0; i < num_particles; i++) { + for (int64_t i = 0; i < NUM_PARTICLES; i++) { /* Recalculate expected values for this iteration */ double expected_x = 0.1 + (double)i * 0.01 + (double)iter_idx * 0.001; double expected_y = 0.2 + (double)i * 0.02 + (double)iter_idx * 0.002; diff --git a/tests/test_write.c b/tests/test_write.c index 2abae50..28699f4 100644 --- a/tests/test_write.c +++ b/tests/test_write.c @@ -35,16 +35,16 @@ static int copy_file(const char *src, const char *dst) { dst_file = fopen(dst, "wb"); if (!dst_file) { - fclose(src_file); + (void)fclose(src_file); return -1; } while ((bytes = fread(buffer, 1, sizeof(buffer), src_file)) > 0) { - fwrite(buffer, 1, bytes, dst_file); + (void)fwrite(buffer, 1, bytes, dst_file); } - fclose(src_file); - fclose(dst_file); + (void)fclose(src_file); + (void)fclose(dst_file); return 0; } @@ -81,7 +81,7 @@ static void remove_directory(const char *path) { if (dir) { while ((entry = readdir(dir)) != NULL) { if (strcmp(entry->d_name, ".") != 0 && strcmp(entry->d_name, "..") != 0) { - snprintf(file_path, sizeof(file_path), "%s/%s", path, entry->d_name); + (void)snprintf(file_path, sizeof(file_path), "%s/%s", path, entry->d_name); struct stat st; if (stat(file_path, &st) == 0) { if (S_ISDIR(st.st_mode)) { @@ -124,7 +124,9 @@ void test_create_group_based_series(void) { TEST_ASSERT_EQUAL_INT(PMD_TRUNC, series->access_mode); TEST_ASSERT_EQUAL_INT(PMD_GROUP_BASED, series->iteration_encoding); - int major, minor, revision; + int major; + int minor; + int revision; result = pmd_get_openpmd_version(series, &major, &minor, &revision); TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, result); TEST_ASSERT_EQUAL_INT(2, major); @@ -144,7 +146,7 @@ void test_create_group_based_series(void) { /* Verify file was created */ FILE *test = fopen(TEST_TEMP_DIR "/test_group.h5", "rb"); TEST_ASSERT_NOT_NULL(test); - fclose(test); + (void)fclose(test); } /** @@ -163,7 +165,9 @@ void test_create_file_based_series(void) { TEST_ASSERT_EQUAL_INT(PMD_TRUNC, series->access_mode); TEST_ASSERT_EQUAL_INT(PMD_FILE_BASED, series->iteration_encoding); - int major_fb, minor_fb, revision_fb; + int major_fb; + int minor_fb; + int revision_fb; result = pmd_get_openpmd_version(series, &major_fb, &minor_fb, &revision_fb); TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, result); TEST_ASSERT_EQUAL_INT(2, major_fb); @@ -208,7 +212,9 @@ void test_open_existing_group_based_rdwr(void) { TEST_ASSERT_EQUAL_INT(PMD_RDWR, series->access_mode); TEST_ASSERT_EQUAL_INT(PMD_GROUP_BASED, series->iteration_encoding); - int major_rdwr, minor_rdwr, revision_rdwr; + int major_rdwr; + int minor_rdwr; + int revision_rdwr; result = pmd_get_openpmd_version(series, &major_rdwr, &minor_rdwr, &revision_rdwr); TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, result); TEST_ASSERT_EQUAL_INT(2, major_rdwr); @@ -300,7 +306,9 @@ void test_create_iteration_group_based(void) { TEST_ASSERT_EQUAL_INT64(0, iter->iteration_index); TEST_ASSERT(iter->iteration_group_id >= 0); - double time, dt, time_unit_si; + double time; + double dt; + double time_unit_si; result = pmd_get_time(iter, &time); TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, result); TEST_ASSERT_EQUAL_DOUBLE(0.0, time); @@ -344,7 +352,8 @@ void test_create_iteration_file_based(void) { TEST_ASSERT(iter->file_id >= 0); TEST_ASSERT(iter->iteration_group_id >= 0); - double time_fb, dt_fb; + double time_fb; + double dt_fb; result = pmd_get_time(iter, &time_fb); TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, result); TEST_ASSERT_EQUAL_DOUBLE(0.0, time_fb); @@ -360,7 +369,7 @@ void test_create_iteration_file_based(void) { /* Verify file was created */ FILE *test = fopen(TEST_TEMP_DIR "/fb_0.h5", "rb"); TEST_ASSERT_NOT_NULL(test); - fclose(test); + (void)fclose(test); /* Close series */ result = pmd_close_series(series); @@ -417,15 +426,15 @@ void test_write_particle_group_minimal(void) { pmd_iteration *iter; pmd_status result; particle_group pg; - const int64_t num_particles = 10; + const int64_t NUM_PARTICLES = 10; /* Allocate position arrays */ - double *x = (double*)malloc(num_particles * sizeof(double)); - double *y = (double*)malloc(num_particles * sizeof(double)); - double *z = (double*)malloc(num_particles * sizeof(double)); + double *x = (double*)malloc(NUM_PARTICLES * sizeof(double)); + double *y = (double*)malloc(NUM_PARTICLES * sizeof(double)); + double *z = (double*)malloc(NUM_PARTICLES * sizeof(double)); /* Initialize with test data */ - for (int64_t i = 0; i < num_particles; i++) { + for (int64_t i = 0; i < NUM_PARTICLES; i++) { x[i] = (double)i * 0.001; y[i] = (double)i * 0.002; z[i] = (double)i * 0.003; @@ -433,7 +442,7 @@ void test_write_particle_group_minimal(void) { /* Setup particle group with minimal fields */ memset(&pg, 0, sizeof(particle_group)); - pg.num_particles = num_particles; + pg.num_particles = NUM_PARTICLES; pg.species_type = "electron"; pg.x = x; pg.y = y; @@ -471,22 +480,22 @@ void test_write_particle_group_complete(void) { pmd_iteration *iter; pmd_status result; particle_group pg; - const int64_t num_particles = 5; + const int64_t NUM_PARTICLES = 5; /* Allocate all arrays */ - double *x = (double*)malloc(num_particles * sizeof(double)); - double *y = (double*)malloc(num_particles * sizeof(double)); - double *z = (double*)malloc(num_particles * sizeof(double)); - double *t = (double*)malloc(num_particles * sizeof(double)); - double *px = (double*)malloc(num_particles * sizeof(double)); - double *py = (double*)malloc(num_particles * sizeof(double)); - double *pz = (double*)malloc(num_particles * sizeof(double)); - double *weight = (double*)malloc(num_particles * sizeof(double)); - int64_t *status = (int64_t*)malloc(num_particles * sizeof(int64_t)); - int64_t *id = (int64_t*)malloc(num_particles * sizeof(int64_t)); + double *x = (double*)malloc(NUM_PARTICLES * sizeof(double)); + double *y = (double*)malloc(NUM_PARTICLES * sizeof(double)); + double *z = (double*)malloc(NUM_PARTICLES * sizeof(double)); + double *t = (double*)malloc(NUM_PARTICLES * sizeof(double)); + double *px = (double*)malloc(NUM_PARTICLES * sizeof(double)); + double *py = (double*)malloc(NUM_PARTICLES * sizeof(double)); + double *pz = (double*)malloc(NUM_PARTICLES * sizeof(double)); + double *weight = (double*)malloc(NUM_PARTICLES * sizeof(double)); + int64_t *status = (int64_t*)malloc(NUM_PARTICLES * sizeof(int64_t)); + int64_t *id = (int64_t*)malloc(NUM_PARTICLES * sizeof(int64_t)); /* Initialize with test data */ - for (int64_t i = 0; i < num_particles; i++) { + for (int64_t i = 0; i < NUM_PARTICLES; i++) { x[i] = (double)i * 0.001; y[i] = (double)i * 0.002; z[i] = (double)i * 0.003; @@ -501,7 +510,7 @@ void test_write_particle_group_complete(void) { /* Setup particle group with all fields */ memset(&pg, 0, sizeof(particle_group)); - pg.num_particles = num_particles; + pg.num_particles = NUM_PARTICLES; pg.species_type = "proton"; pg.x = x; pg.y = y; @@ -920,15 +929,15 @@ void test_write_nonconsecutive_iterations_file_based(void) { /* Verify individual files were created */ FILE *test1 = fopen(TEST_TEMP_DIR "/nonconsec_1.h5", "rb"); TEST_ASSERT_NOT_NULL(test1); - fclose(test1); + (void)fclose(test1); FILE *test3 = fopen(TEST_TEMP_DIR "/nonconsec_3.h5", "rb"); TEST_ASSERT_NOT_NULL(test3); - fclose(test3); + (void)fclose(test3); FILE *test7 = fopen(TEST_TEMP_DIR "/nonconsec_7.h5", "rb"); TEST_ASSERT_NOT_NULL(test7); - fclose(test7); + (void)fclose(test7); /* Reopen with pattern and verify iterations */ result = pmd_open_series(TEST_TEMP_DIR "/nonconsec_%T.h5", &series, PMD_RDONLY); @@ -1048,7 +1057,7 @@ void test_valid_filebased_patterns(void) { /* Verify file was created at expected location */ FILE *test = fopen(test_cases[i].expected_file, "rb"); TEST_ASSERT_NOT_NULL_MESSAGE(test, test_cases[i].description); - if (test) fclose(test); + if (test) (void)fclose(test); /* Reopen series and verify we can read it back */ result = pmd_open_series(test_cases[i].pattern, &series, PMD_RDONLY); @@ -1090,17 +1099,17 @@ void test_openpmd_required_attributes(void) { particle_group pg; int64_t test_iterations[] = {0, 5, 10, 14, 23, 45, 90, 100, 1024, 10920}; int num_test_iters = 10; - const int64_t num_particles = 5; + const int64_t NUM_PARTICLES = 5; /* Allocate particle data */ - double *x = (double*)malloc(num_particles * sizeof(double)); - double *y = (double*)malloc(num_particles * sizeof(double)); - double *z = (double*)malloc(num_particles * sizeof(double)); - double *px = (double*)malloc(num_particles * sizeof(double)); - double *py = (double*)malloc(num_particles * sizeof(double)); - double *pz = (double*)malloc(num_particles * sizeof(double)); - - for (int64_t i = 0; i < num_particles; i++) { + double *x = (double*)malloc(NUM_PARTICLES * sizeof(double)); + double *y = (double*)malloc(NUM_PARTICLES * sizeof(double)); + double *z = (double*)malloc(NUM_PARTICLES * sizeof(double)); + double *px = (double*)malloc(NUM_PARTICLES * sizeof(double)); + double *py = (double*)malloc(NUM_PARTICLES * sizeof(double)); + double *pz = (double*)malloc(NUM_PARTICLES * sizeof(double)); + + for (int64_t i = 0; i < NUM_PARTICLES; i++) { x[i] = (double)i * 0.001; y[i] = (double)i * 0.002; z[i] = (double)i * 0.003; @@ -1111,7 +1120,7 @@ void test_openpmd_required_attributes(void) { /* Setup particle group */ memset(&pg, 0, sizeof(particle_group)); - pg.num_particles = num_particles; + pg.num_particles = NUM_PARTICLES; pg.species_type = "electron"; pg.x = x; pg.y = y; @@ -1249,7 +1258,7 @@ void test_openpmd_required_attributes(void) { * Test: File-based pattern fails if parent directory before %T doesn't exist * Parcel should only create directories it's responsible for (containing %T) */ -void test_filebased_fails_parent_before_T_missing(void) { +void test_filebased_fails_parent_before_t_missing(void) { pmd_series *series; pmd_iteration *iter; pmd_status result; @@ -1271,14 +1280,14 @@ void test_write_particle_group_errors(void) { pmd_iteration *iter; pmd_status result; particle_group pg; - const int64_t num_particles = 2; + const int64_t NUM_PARTICLES = 2; - double *x = (double*)malloc(num_particles * sizeof(double)); - double *y = (double*)malloc(num_particles * sizeof(double)); - double *z = (double*)malloc(num_particles * sizeof(double)); + double *x = (double*)malloc(NUM_PARTICLES * sizeof(double)); + double *y = (double*)malloc(NUM_PARTICLES * sizeof(double)); + double *z = (double*)malloc(NUM_PARTICLES * sizeof(double)); /* Initialize */ - for (int64_t i = 0; i < num_particles; i++) { + for (int64_t i = 0; i < NUM_PARTICLES; i++) { x[i] = (double)i; y[i] = (double)i; z[i] = (double)i; @@ -1286,7 +1295,7 @@ void test_write_particle_group_errors(void) { /* Test: NULL species_type */ memset(&pg, 0, sizeof(particle_group)); - pg.num_particles = num_particles; + pg.num_particles = NUM_PARTICLES; pg.species_type = NULL; /* Missing */ pg.x = x; pg.y = y; @@ -1326,7 +1335,7 @@ void test_write_particle_group_errors(void) { result = pmd_open_iteration(series, 0, &iter); TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, result); - pg.num_particles = num_particles; + pg.num_particles = NUM_PARTICLES; result = pmd_write_particle_group(iter, &pg); TEST_ASSERT_EQUAL_INT(PMD_ERROR, result); /* Should fail - read-only */ @@ -1458,8 +1467,8 @@ void test_set_metadata_file_based(void) { /* Reopen and verify attributes in all iteration files */ for (int i = 0; i < 3; i++) { char filename[256]; - snprintf(filename, sizeof(filename), TEST_TEMP_DIR "/metadata_fb_%lld.h5", - (long long)test_iterations[i]); + (void)snprintf(filename, sizeof(filename), TEST_TEMP_DIR "/metadata_fb_%lld.h5", + (long long)test_iterations[i]); hid_t file_id = H5Fopen(filename, H5F_ACC_RDONLY, H5P_DEFAULT); TEST_ASSERT_MESSAGE(file_id >= 0, filename); @@ -1793,7 +1802,8 @@ void test_metadata_changes_propagate_file_based(void) { */ void test_metadata_write_with_open_iterations(void) { pmd_series *series; - pmd_iteration *iter0, *iter1; + pmd_iteration *iter0; + pmd_iteration *iter1; pmd_status result; char *value; @@ -2018,15 +2028,15 @@ void test_trunc_deletes_existing_file_based(void) { /* Verify files exist */ FILE *f0 = fopen(TEST_TEMP_DIR "/trunc_test_0.h5", "rb"); TEST_ASSERT_NOT_NULL_MESSAGE(f0, "Iteration 0 file should exist"); - fclose(f0); + (void)fclose(f0); FILE *f1 = fopen(TEST_TEMP_DIR "/trunc_test_1.h5", "rb"); TEST_ASSERT_NOT_NULL_MESSAGE(f1, "Iteration 1 file should exist"); - fclose(f1); + (void)fclose(f1); FILE *f2 = fopen(TEST_TEMP_DIR "/trunc_test_2.h5", "rb"); TEST_ASSERT_NOT_NULL_MESSAGE(f2, "Iteration 2 file should exist"); - fclose(f2); + (void)fclose(f2); /* Now reopen with TRUNC mode - should delete existing files */ result = pmd_open_series(TEST_TEMP_DIR "/trunc_test_%T.h5", &series, PMD_TRUNC); @@ -2072,11 +2082,11 @@ void test_trunc_deletes_existing_file_based(void) { /* Verify new files exist */ FILE *f5 = fopen(TEST_TEMP_DIR "/trunc_test_5.h5", "rb"); TEST_ASSERT_NOT_NULL_MESSAGE(f5, "Iteration 5 file should exist"); - fclose(f5); + (void)fclose(f5); FILE *f10 = fopen(TEST_TEMP_DIR "/trunc_test_10.h5", "rb"); TEST_ASSERT_NOT_NULL_MESSAGE(f10, "Iteration 10 file should exist"); - fclose(f10); + (void)fclose(f10); /* Old files should still not exist */ f0 = fopen(TEST_TEMP_DIR "/trunc_test_0.h5", "rb"); @@ -2089,7 +2099,11 @@ void test_trunc_deletes_existing_file_based(void) { */ void test_open_iteration_tracking_file_based(void) { pmd_series *series; - pmd_iteration *iter0, *iter1, *iter2, *iter3, *iter4; + pmd_iteration *iter0; + pmd_iteration *iter1; + pmd_iteration *iter2; + pmd_iteration *iter3; + pmd_iteration *iter4; pmd_iteration *iter0_reopen; pmd_status result; @@ -2181,37 +2195,38 @@ void test_species_info_after_write(void) { pmd_series *series; pmd_iteration *iter; pmd_status result; - particle_group pg_electron, pg_proton; - const int64_t num_electrons = 100; - const int64_t num_protons = 50; + particle_group pg_electron; + particle_group pg_proton; + const int64_t NUM_ELECTRONS = 100; + const int64_t NUM_PROTONS = 50; /* Allocate electron arrays */ - double *e_x = (double*)calloc(num_electrons, sizeof(double)); - double *e_y = (double*)calloc(num_electrons, sizeof(double)); - double *e_z = (double*)calloc(num_electrons, sizeof(double)); - double *e_px = (double*)calloc(num_electrons, sizeof(double)); - double *e_py = (double*)calloc(num_electrons, sizeof(double)); - double *e_pz = (double*)calloc(num_electrons, sizeof(double)); - double *e_t = (double*)calloc(num_electrons, sizeof(double)); - double *e_weight = (double*)calloc(num_electrons, sizeof(double)); - int64_t *e_status = (int64_t*)calloc(num_electrons, sizeof(int64_t)); - int64_t *e_id = (int64_t*)calloc(num_electrons, sizeof(int64_t)); + double *e_x = (double*)calloc(NUM_ELECTRONS, sizeof(double)); + double *e_y = (double*)calloc(NUM_ELECTRONS, sizeof(double)); + double *e_z = (double*)calloc(NUM_ELECTRONS, sizeof(double)); + double *e_px = (double*)calloc(NUM_ELECTRONS, sizeof(double)); + double *e_py = (double*)calloc(NUM_ELECTRONS, sizeof(double)); + double *e_pz = (double*)calloc(NUM_ELECTRONS, sizeof(double)); + double *e_t = (double*)calloc(NUM_ELECTRONS, sizeof(double)); + double *e_weight = (double*)calloc(NUM_ELECTRONS, sizeof(double)); + int64_t *e_status = (int64_t*)calloc(NUM_ELECTRONS, sizeof(int64_t)); + int64_t *e_id = (int64_t*)calloc(NUM_ELECTRONS, sizeof(int64_t)); /* Allocate proton arrays */ - double *p_x = (double*)calloc(num_protons, sizeof(double)); - double *p_y = (double*)calloc(num_protons, sizeof(double)); - double *p_z = (double*)calloc(num_protons, sizeof(double)); - double *p_px = (double*)calloc(num_protons, sizeof(double)); - double *p_py = (double*)calloc(num_protons, sizeof(double)); - double *p_pz = (double*)calloc(num_protons, sizeof(double)); - double *p_t = (double*)calloc(num_protons, sizeof(double)); - double *p_weight = (double*)calloc(num_protons, sizeof(double)); - int64_t *p_status = (int64_t*)calloc(num_protons, sizeof(int64_t)); - int64_t *p_id = (int64_t*)calloc(num_protons, sizeof(int64_t)); + double *p_x = (double*)calloc(NUM_PROTONS, sizeof(double)); + double *p_y = (double*)calloc(NUM_PROTONS, sizeof(double)); + double *p_z = (double*)calloc(NUM_PROTONS, sizeof(double)); + double *p_px = (double*)calloc(NUM_PROTONS, sizeof(double)); + double *p_py = (double*)calloc(NUM_PROTONS, sizeof(double)); + double *p_pz = (double*)calloc(NUM_PROTONS, sizeof(double)); + double *p_t = (double*)calloc(NUM_PROTONS, sizeof(double)); + double *p_weight = (double*)calloc(NUM_PROTONS, sizeof(double)); + int64_t *p_status = (int64_t*)calloc(NUM_PROTONS, sizeof(int64_t)); + int64_t *p_id = (int64_t*)calloc(NUM_PROTONS, sizeof(int64_t)); /* Initialize electron particle group */ memset(&pg_electron, 0, sizeof(particle_group)); - pg_electron.num_particles = num_electrons; + pg_electron.num_particles = NUM_ELECTRONS; pg_electron.species_type = "electron"; pg_electron.x = e_x; pg_electron.y = e_y; @@ -2226,7 +2241,7 @@ void test_species_info_after_write(void) { /* Initialize proton particle group */ memset(&pg_proton, 0, sizeof(particle_group)); - pg_proton.num_particles = num_protons; + pg_proton.num_particles = NUM_PROTONS; pg_proton.species_type = "proton"; pg_proton.x = p_x; pg_proton.y = p_y; @@ -2261,7 +2276,8 @@ void test_species_info_after_write(void) { TEST_ASSERT_EQUAL_INT(2, species_count); /* Species should be electron and proton (order may vary) */ - int found_electron = 0, found_proton = 0; + int found_electron = 0; + int found_proton = 0; for (int i = 0; i < species_count; i++) { if (strcmp(species_names[i], "electron") == 0) { found_electron = 1; @@ -2269,14 +2285,14 @@ void test_species_info_after_write(void) { int64_t count; result = pmd_get_num_particles(iter, "electron", &count); TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, result); - TEST_ASSERT_EQUAL_INT64(num_electrons, count); + TEST_ASSERT_EQUAL_INT64(NUM_ELECTRONS, count); } else if (strcmp(species_names[i], "proton") == 0) { found_proton = 1; /* Verify particle count for protons */ int64_t count; result = pmd_get_num_particles(iter, "proton", &count); TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, result); - TEST_ASSERT_EQUAL_INT64(num_protons, count); + TEST_ASSERT_EQUAL_INT64(NUM_PROTONS, count); } free(species_names[i]); } @@ -2434,7 +2450,7 @@ int main(void) { RUN_TEST(test_write_fails_no_parent_directory); RUN_TEST(test_invalid_pattern_ambiguous); RUN_TEST(test_valid_filebased_patterns); - RUN_TEST(test_filebased_fails_parent_before_T_missing); + RUN_TEST(test_filebased_fails_parent_before_t_missing); RUN_TEST(test_openpmd_required_attributes); RUN_TEST(test_write_particle_group_minimal); RUN_TEST(test_write_particle_group_complete); From 02197e54f9296d12eafc89b3718152f811af7346 Mon Sep 17 00:00:00 2001 From: "Christopher M. Pierce" Date: Mon, 5 Jan 2026 21:02:34 -0500 Subject: [PATCH 09/29] more clang-tidy cleanup --- tests/test_generate_openpmd.c | 60 +++++++++++++++++------------------ 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/tests/test_generate_openpmd.c b/tests/test_generate_openpmd.c index 0c8d484..8895188 100644 --- a/tests/test_generate_openpmd.c +++ b/tests/test_generate_openpmd.c @@ -71,24 +71,24 @@ void test_generate_group_based_openpmd(void) { pmd_iteration *iter; pmd_status result; particle_group pg; - const int64_t num_particles = 10; - const int64_t num_iterations = 3; + const int64_t NUM_PARTICLES = 10; + const int64_t NUM_ITERATIONS = 3; /* Allocate particle arrays */ - double *x = (double*)malloc(num_particles * sizeof(double)); - double *y = (double*)malloc(num_particles * sizeof(double)); - double *z = (double*)malloc(num_particles * sizeof(double)); - double *t = (double*)malloc(num_particles * sizeof(double)); - double *px = (double*)malloc(num_particles * sizeof(double)); - double *py = (double*)malloc(num_particles * sizeof(double)); - double *pz = (double*)malloc(num_particles * sizeof(double)); - double *weight = (double*)malloc(num_particles * sizeof(double)); - int64_t *status = (int64_t*)malloc(num_particles * sizeof(int64_t)); - int64_t *id = (int64_t*)malloc(num_particles * sizeof(int64_t)); + double *x = (double*)malloc(NUM_PARTICLES * sizeof(double)); + double *y = (double*)malloc(NUM_PARTICLES * sizeof(double)); + double *z = (double*)malloc(NUM_PARTICLES * sizeof(double)); + double *t = (double*)malloc(NUM_PARTICLES * sizeof(double)); + double *px = (double*)malloc(NUM_PARTICLES * sizeof(double)); + double *py = (double*)malloc(NUM_PARTICLES * sizeof(double)); + double *pz = (double*)malloc(NUM_PARTICLES * sizeof(double)); + double *weight = (double*)malloc(NUM_PARTICLES * sizeof(double)); + int64_t *status = (int64_t*)malloc(NUM_PARTICLES * sizeof(int64_t)); + int64_t *id = (int64_t*)malloc(NUM_PARTICLES * sizeof(int64_t)); /* Setup particle group structure */ memset(&pg, 0, sizeof(particle_group)); - pg.num_particles = num_particles; + pg.num_particles = NUM_PARTICLES; pg.species_type = "electron"; pg.x = x; pg.y = y; @@ -106,11 +106,11 @@ void test_generate_group_based_openpmd(void) { TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, result); /* Write multiple iterations with non-consecutive indices (0, 5, 10) */ - for (int64_t iter_loop = 0; iter_loop < num_iterations; iter_loop++) { + for (int64_t iter_loop = 0; iter_loop < NUM_ITERATIONS; iter_loop++) { int64_t iter_idx = iter_loop * 5; /* Generate iteration-specific particle data */ - generate_particle_data(iter_idx, num_particles, &pg); + generate_particle_data(iter_idx, NUM_PARTICLES, &pg); result = pmd_open_iteration(series, iter_idx, &iter); TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, result); @@ -147,28 +147,28 @@ void test_generate_file_based_openpmd(void) { pmd_iteration *iter; pmd_status result; particle_group pg; - const int64_t num_particles = 10; - const int64_t num_iterations = 3; + const int64_t NUM_PARTICLES = 10; + const int64_t NUM_ITERATIONS = 3; const char *output_dir = TEST_OUTPUT_DIR "/file_based_example"; /* Create subdirectory for file-based iterations */ mkdir(output_dir, 0755); /* Allocate particle arrays */ - double *x = (double*)malloc(num_particles * sizeof(double)); - double *y = (double*)malloc(num_particles * sizeof(double)); - double *z = (double*)malloc(num_particles * sizeof(double)); - double *t = (double*)malloc(num_particles * sizeof(double)); - double *px = (double*)malloc(num_particles * sizeof(double)); - double *py = (double*)malloc(num_particles * sizeof(double)); - double *pz = (double*)malloc(num_particles * sizeof(double)); - double *weight = (double*)malloc(num_particles * sizeof(double)); - int64_t *status = (int64_t*)malloc(num_particles * sizeof(int64_t)); - int64_t *id = (int64_t*)malloc(num_particles * sizeof(int64_t)); + double *x = (double*)malloc(NUM_PARTICLES * sizeof(double)); + double *y = (double*)malloc(NUM_PARTICLES * sizeof(double)); + double *z = (double*)malloc(NUM_PARTICLES * sizeof(double)); + double *t = (double*)malloc(NUM_PARTICLES * sizeof(double)); + double *px = (double*)malloc(NUM_PARTICLES * sizeof(double)); + double *py = (double*)malloc(NUM_PARTICLES * sizeof(double)); + double *pz = (double*)malloc(NUM_PARTICLES * sizeof(double)); + double *weight = (double*)malloc(NUM_PARTICLES * sizeof(double)); + int64_t *status = (int64_t*)malloc(NUM_PARTICLES * sizeof(int64_t)); + int64_t *id = (int64_t*)malloc(NUM_PARTICLES * sizeof(int64_t)); /* Setup particle group structure */ memset(&pg, 0, sizeof(particle_group)); - pg.num_particles = num_particles; + pg.num_particles = NUM_PARTICLES; pg.species_type = "electron"; pg.x = x; pg.y = y; @@ -187,11 +187,11 @@ void test_generate_file_based_openpmd(void) { TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, result); /* Write multiple iterations with non-consecutive indices (0, 5, 10) */ - for (int64_t iter_loop = 0; iter_loop < num_iterations; iter_loop++) { + for (int64_t iter_loop = 0; iter_loop < NUM_ITERATIONS; iter_loop++) { int64_t iter_idx = iter_loop * 5; /* Generate iteration-specific particle data */ - generate_particle_data(iter_idx, num_particles, &pg); + generate_particle_data(iter_idx, NUM_PARTICLES, &pg); result = pmd_open_iteration(series, iter_idx, &iter); TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, result); From d0febbffe9a6837e16aadb513d42bace7e84f443 Mon Sep 17 00:00:00 2001 From: "Christopher M. Pierce" Date: Mon, 5 Jan 2026 21:27:41 -0500 Subject: [PATCH 10/29] more cleaning up control flow --- parcel.h | 83 +++++++++++++++++++++++++------------------------------- 1 file changed, 37 insertions(+), 46 deletions(-) diff --git a/parcel.h b/parcel.h index b7bf585..ff71bc2 100644 --- a/parcel.h +++ b/parcel.h @@ -1703,23 +1703,32 @@ pmd_status pmd_open_series(const char *filename, pmd_series **series_out, pmd_ac hid_t file_id = -1; pmd_status status = PMD_SUCCESS; char *actual_filename = NULL; + char *full_path = NULL; int is_write_mode = (mode != PMD_RDONLY); int file_exists = 0; + iteration_pattern pattern_info; + pattern_info.scan_parent = NULL; + pattern_info.first_segment = NULL; + pattern_info.full_pattern = NULL; + /* Validate input */ if (!filename || !series_out) { - return PMD_ERROR_NULL_POINTER; + status = PMD_ERROR_NULL_POINTER; + goto cleanup; } /* Check if parent directory exists */ if (!parent_directory_exists(filename)) { - return PMD_ERROR_FILE_NOT_FOUND; + status = PMD_ERROR_FILE_NOT_FOUND; + goto cleanup; } /* Allocate series struct */ series = (pmd_series *)calloc(1, sizeof(pmd_series)); if (!series) { - return PMD_ERROR_OUT_OF_MEMORY; + status = PMD_ERROR_OUT_OF_MEMORY; + goto cleanup; } /* Initialize fields */ @@ -1746,20 +1755,14 @@ pmd_status pmd_open_series(const char *filename, pmd_series **series_out, pmd_ac /* Pattern-based filename */ if (strstr(filename, "%T") != NULL) { /* For pattern-based filename, first check if any matching files exist */ - iteration_pattern pattern_info; status = parse_iteration_pattern(filename, &pattern_info); - if (status != PMD_SUCCESS) { - free(series); - return status; - } + if (status != PMD_SUCCESS) goto cleanup; /* Open directory and search for matching files */ pmd_dir *dir = pmd_opendir(pattern_info.scan_parent); if (!dir) { - /* Read mode requires existing files */ - free_iteration_pattern(&pattern_info); - free(series); - return PMD_ERROR_FILE_NOT_FOUND; + status = PMD_ERROR_FILE_NOT_FOUND; + goto cleanup; } /* Find first file matching the pattern */ @@ -1770,15 +1773,17 @@ pmd_status pmd_open_series(const char *filename, pmd_series **series_out, pmd_ac int64_t iteration; /* Try to extract iteration from name matching first segment pattern */ if (extract_iteration_from_name(entry->d_name, pattern_info.first_segment, &iteration) == PMD_SUCCESS) { - /* Reconstruct full file path from pattern */ - char *full_path = replace_iteration(filename, iteration); + /* Reconstruct full file path from pattern. We are allocating memory in a loop, free here if */ + /* memory was allocated before allocating more memory to avoid leak. */ + free(full_path); + full_path = replace_iteration(filename, iteration); if (!full_path) { pmd_closedir(dir); - free_iteration_pattern(&pattern_info); - free(series); - return PMD_ERROR_OUT_OF_MEMORY; + status = PMD_ERROR_OUT_OF_MEMORY; + goto cleanup; } + // Open the HDF5 file file_id = H5Fopen(full_path, H5F_ACC_RDONLY, H5P_DEFAULT); if (file_id >= 0) { actual_filename = full_path; @@ -1793,8 +1798,6 @@ pmd_status pmd_open_series(const char *filename, pmd_series **series_out, pmd_ac /* Close file since we will not store for file-based mode */ H5Fclose(file_id); file_id = -1; - } else { - free(full_path); } } } @@ -1804,10 +1807,8 @@ pmd_status pmd_open_series(const char *filename, pmd_series **series_out, pmd_ac if (!found) { /* No existing files found */ if (!is_write_mode) { - /* Read mode requires existing files */ - free_iteration_pattern(&pattern_info); - free(series); - return PMD_ERROR_FILE_NOT_FOUND; + status = PMD_ERROR_FILE_NOT_FOUND; + goto cleanup; } /* Write mode - creating new file-based series */ @@ -1915,8 +1916,8 @@ pmd_status pmd_open_series(const char *filename, pmd_series **series_out, pmd_ac unsigned int h5_flags = pmd_access_mode_to_hdf5(mode); file_id = H5Fcreate(filename, h5_flags, H5P_DEFAULT, H5P_DEFAULT); if (file_id < 0) { - free(series); - return PMD_ERROR_HDF5; + status = PMD_ERROR_HDF5; + goto cleanup; } /* Set up group-based encoding defaults */ @@ -1927,11 +1928,7 @@ pmd_status pmd_open_series(const char *filename, pmd_series **series_out, pmd_ac /* Write required attributes */ status = write_root_attributes(file_id, series); - if (status != PMD_SUCCESS) { - H5Fclose(file_id); - free(series); - return status; - } + if (status != PMD_SUCCESS) goto cleanup; /* Create base path structure up to scan_parent for GROUP_BASED */ /* This ensures pmd_get_iterations can enumerate groups even when no iterations exist yet */ @@ -1974,15 +1971,15 @@ pmd_status pmd_open_series(const char *filename, pmd_series **series_out, pmd_ac /* Opening existing file (PMD_RDONLY or PMD_RDWR) */ if (!file_exists) { - free(series); - return PMD_ERROR_FILE_NOT_FOUND; + status = PMD_ERROR_FILE_NOT_FOUND; + goto cleanup; } unsigned int h5_flags = pmd_access_mode_to_hdf5(mode); file_id = H5Fopen(filename, h5_flags, H5P_DEFAULT); if (file_id < 0) { - free(series); - return PMD_ERROR_HDF5; + status = PMD_ERROR_HDF5; + goto cleanup; } actual_filename = strdup(filename); series->file_id = file_id; @@ -2030,35 +2027,29 @@ pmd_status pmd_open_series(const char *filename, pmd_series **series_out, pmd_ac } } - free(actual_filename); - /* Write root attributes if in write mode */ if (is_write_mode) { status = write_series_root_attributes(series); - if (status != PMD_SUCCESS) { - pmd_close_series(series); - return status; - } + if (status != PMD_SUCCESS) goto cleanup; } - /* Final check - if series initialized successfully, return it */ - *series_out = series; - return PMD_SUCCESS; - cleanup: if (file_id >= 0 && series->file_id < 0) { H5Fclose(file_id); } - if (series) { + if (status != PMD_SUCCESS) { pmd_close_series(series); + series = NULL; } free(actual_filename); + free_iteration_pattern(&pattern_info); + *series_out = series; return status; } pmd_status pmd_close_series(pmd_series *series) { if (!series) { - return PMD_ERROR_NULL_POINTER; + return PMD_SUCCESS; } /* Close file if open */ From f6dfc66848c5aebee108635ca9d940f2c3e6512a Mon Sep 17 00:00:00 2001 From: "Christopher M. Pierce" Date: Mon, 5 Jan 2026 22:05:18 -0500 Subject: [PATCH 11/29] more cleanup --- parcel.h | 303 +++++++++++++++++++++++----------------------- tests/test_read.c | 2 + 2 files changed, 154 insertions(+), 151 deletions(-) diff --git a/parcel.h b/parcel.h index ff71bc2..31b2d8f 100644 --- a/parcel.h +++ b/parcel.h @@ -716,6 +716,7 @@ static pmd_status write_int64_dataset(hid_t group_id, const char *name, const in int64_t num_particles, const pmd_unit_dimension *unit_dim, double time_offset); static pmd_status write_int64_attribute(hid_t loc_id, const char *attr_name, int64_t value); +static void pmd_invalidate_iterations_cache(pmd_series *series); /* Pattern matching forward declarations */ typedef struct { @@ -2157,168 +2158,183 @@ static herr_t collect_iterations_callback(hid_t loc_id, const char *name, } pmd_status pmd_get_iterations(pmd_series *series, int64_t **iterations, int *count) { + pmd_status status = PMD_SUCCESS; + iteration_pattern pattern_info; + pattern_info.scan_parent = NULL; + pattern_info.first_segment = NULL; + pattern_info.full_pattern = NULL; + if (!series || !iterations || !count) { - return PMD_ERROR_NULL_POINTER; + status = PMD_ERROR_NULL_POINTER; + goto cleanup; } - /* If already enumerated, return cached results */ - if (series->num_iterations >= 0) { - *iterations = series->iteration_indices; - *count = series->num_iterations; - return PMD_SUCCESS; - } + /* Fetch the iteration results if we must */ + if (series->num_iterations < 0) { + /* Check for single-snapshot file (no %T in iteration_format) */ + if (!strstr(series->iteration_format, "%T")) { + /* Single iteration at index 0 */ + series->iteration_indices = (int64_t *)malloc(sizeof(int64_t)); + if (!series->iteration_indices) { + return PMD_ERROR_OUT_OF_MEMORY; + } + series->iteration_indices[0] = 0; + series->num_iterations = 1; + *iterations = series->iteration_indices; + *count = 1; + return PMD_SUCCESS; + } - /* Check for single-snapshot file (no %T in iteration_format) */ - if (!strstr(series->iteration_format, "%T")) { - /* Single iteration at index 0 */ - series->iteration_indices = (int64_t *)malloc(sizeof(int64_t)); - if (!series->iteration_indices) { - return PMD_ERROR_OUT_OF_MEMORY; + /* Parse iteration_format for both GROUP_BASED and FILE_BASED */ + pmd_status status = parse_iteration_pattern(series->iteration_format, &pattern_info); + if (status != PMD_SUCCESS) { + return status; } - series->iteration_indices[0] = 0; - series->num_iterations = 1; - *iterations = series->iteration_indices; - *count = 1; - return PMD_SUCCESS; - } - /* Parse iteration_format for both GROUP_BASED and FILE_BASED */ - iteration_pattern pattern_info; - pmd_status status = parse_iteration_pattern(series->iteration_format, &pattern_info); - if (status != PMD_SUCCESS) { - return status; - } + /* Common collector for both GROUP_BASED and FILE_BASED */ + iteration_collector collector = {NULL, 0, 0, NULL, NULL, -1, PMD_SUCCESS}; - /* Common collector for both GROUP_BASED and FILE_BASED */ - iteration_collector collector = {NULL, 0, 0, NULL, NULL, -1, PMD_SUCCESS}; + /* Initialize collector with pattern info */ + collector.first_segment = pattern_info.first_segment; + collector.full_pattern = pattern_info.full_pattern; - /* Initialize collector with pattern info */ - collector.first_segment = pattern_info.first_segment; - collector.full_pattern = pattern_info.full_pattern; + if (series->iteration_encoding == PMD_GROUP_BASED) { - if (series->iteration_encoding == PMD_GROUP_BASED) { + /* GROUP_BASED: enumerate groups using pattern matching */ + collector.root_id = series->file_id; - /* GROUP_BASED: enumerate groups using pattern matching */ - collector.root_id = series->file_id; + /* Open parent group (scan_parent is "." for root) */ + if (record_exists(series->file_id, pattern_info.scan_parent) < 1) { + free_iteration_pattern(&pattern_info); + return PMD_ERROR_FILE_FORMAT; + } + hid_t group_id = H5Gopen(series->file_id, pattern_info.scan_parent, H5P_DEFAULT); - /* Open parent group (scan_parent is "." for root) */ - if (record_exists(series->file_id, pattern_info.scan_parent) < 1) { - free_iteration_pattern(&pattern_info); - return PMD_ERROR_FILE_FORMAT; - } - hid_t group_id = H5Gopen(series->file_id, pattern_info.scan_parent, H5P_DEFAULT); + if (group_id < 0) { + free_iteration_pattern(&pattern_info); + return PMD_ERROR_HDF5; + } - if (group_id < 0) { - free_iteration_pattern(&pattern_info); - return PMD_ERROR_HDF5; - } + /* Iterate through groups to find iterations */ + herr_t iter_result = H5Literate(group_id, H5_INDEX_NAME, H5_ITER_NATIVE, NULL, + collect_iterations_callback, &collector); - /* Iterate through groups to find iterations */ - herr_t iter_result = H5Literate(group_id, H5_INDEX_NAME, H5_ITER_NATIVE, NULL, - collect_iterations_callback, &collector); + H5Gclose(group_id); - H5Gclose(group_id); + /* Check if iteration callback encountered an error */ + if (iter_result < 0 && collector.status != PMD_SUCCESS) { + free(collector.indices); + free_iteration_pattern(&pattern_info); + return collector.status; + } - /* Check if iteration callback encountered an error */ - if (iter_result < 0 && collector.status != PMD_SUCCESS) { - free(collector.indices); - free_iteration_pattern(&pattern_info); - return collector.status; - } + } else { /* PMD_FILE_BASED */ - } else { /* PMD_FILE_BASED */ + /* FILE_BASED: scan directory for matching files using pattern matching */ + /* For FILE_BASED, scan_parent is always "." (validated in pmd_open_series) */ - /* FILE_BASED: scan directory for matching files using pattern matching */ - /* For FILE_BASED, scan_parent is always "." (validated in pmd_open_series) */ + pmd_dir *dir = pmd_opendir(series->directory); + if (!dir) { + free_iteration_pattern(&pattern_info); + return PMD_ERROR_FILE_NOT_FOUND; + } - pmd_dir *dir = pmd_opendir(series->directory); - if (!dir) { - free_iteration_pattern(&pattern_info); - return PMD_ERROR_FILE_NOT_FOUND; - } + pmd_dirent *entry; - pmd_dirent *entry; + /* Scan directory for matching files/directories */ + while ((entry = pmd_readdir(dir)) != NULL) { + int64_t iteration; - /* Scan directory for matching files/directories */ - while ((entry = pmd_readdir(dir)) != NULL) { - int64_t iteration; + /* Try to extract iteration from name matching first segment pattern */ + if (extract_iteration_from_name(entry->d_name, pattern_info.first_segment, &iteration) != PMD_SUCCESS) { + continue; /* Name doesn't match, skip */ + } - /* Try to extract iteration from name matching first segment pattern */ - if (extract_iteration_from_name(entry->d_name, pattern_info.first_segment, &iteration) != PMD_SUCCESS) { - continue; /* Name doesn't match, skip */ - } + /* Validate full path if pattern has additional path components */ + if (pattern_info.full_pattern && (strchr(pattern_info.full_pattern, '/') || strchr(pattern_info.full_pattern, '\\'))) { + int status; - /* Validate full path if pattern has additional path components */ - if (pattern_info.full_pattern && (strchr(pattern_info.full_pattern, '/') || strchr(pattern_info.full_pattern, '\\'))) { - int status; + /* Build full file path with iteration substituted */ + char *rel_path = replace_iteration(series->iteration_format, iteration); + if (!rel_path) { + collector.status = PMD_ERROR_OUT_OF_MEMORY; + break; + } - /* Build full file path with iteration substituted */ - char *rel_path = replace_iteration(series->iteration_format, iteration); - if (!rel_path) { - collector.status = PMD_ERROR_OUT_OF_MEMORY; - break; - } + char full_path[PMD_PATH_MAX]; + status = snprintf(full_path, sizeof(full_path), "%s" PMD_PATH_SEP "%s", series->directory, rel_path); + if (status < 0) { + pmd_log(PMD_LOG_ERROR, "pmd_get_iterations - failed to construct full path."); + return PMD_ERROR; + } + free(rel_path); - char full_path[PMD_PATH_MAX]; - status = snprintf(full_path, sizeof(full_path), "%s" PMD_PATH_SEP "%s", series->directory, rel_path); - if (status < 0) { - pmd_log(PMD_LOG_ERROR, "pmd_get_iterations - failed to construct full path."); - return PMD_ERROR; + /* Check if file exists using fopen (standard C) */ + FILE *test_file = fopen(full_path, "r"); + if (!test_file) { + continue; /* Path doesn't exist, skip */ + } + status = fclose(test_file); + if (status < 0){ + pmd_log(PMD_LOG_ERROR, "pmd_get_iterations - failed to close test file"); + return PMD_ERROR; + } } - free(rel_path); - /* Check if file exists using fopen (standard C) */ - FILE *test_file = fopen(full_path, "r"); - if (!test_file) { - continue; /* Path doesn't exist, skip */ - } - status = fclose(test_file); - if (status < 0){ - pmd_log(PMD_LOG_ERROR, "pmd_get_iterations - failed to close test file"); - return PMD_ERROR; + /* Grow array if needed */ + if (collector.count >= collector.capacity) { + collector.capacity = collector.capacity * 2 + 10; + int64_t *temp = (int64_t *)realloc(collector.indices, + collector.capacity * sizeof(int64_t)); + if (!temp) { + collector.status = PMD_ERROR_OUT_OF_MEMORY; + break; + } + collector.indices = temp; } + + collector.indices[collector.count++] = iteration; } - /* Grow array if needed */ - if (collector.count >= collector.capacity) { - collector.capacity = collector.capacity * 2 + 10; - int64_t *temp = (int64_t *)realloc(collector.indices, - collector.capacity * sizeof(int64_t)); - if (!temp) { - collector.status = PMD_ERROR_OUT_OF_MEMORY; - break; - } - collector.indices = temp; + pmd_closedir(dir); + + /* Check for errors during collection */ + if (collector.status != PMD_SUCCESS) { + free(collector.indices); + free_iteration_pattern(&pattern_info); + return collector.status; } + } - collector.indices[collector.count++] = iteration; + /* Sort the iterations */ + if (collector.count > 0) { + qsort(collector.indices, collector.count, sizeof(int64_t), compare_int64); } - pmd_closedir(dir); + /* Cache results */ + series->iteration_indices = collector.indices; + series->num_iterations = collector.count; + *iterations = collector.indices; + *count = collector.count; + } - /* Check for errors during collection */ - if (collector.status != PMD_SUCCESS) { - free(collector.indices); - free_iteration_pattern(&pattern_info); - return collector.status; - } + /* Populate the user fields */ + *count = series->num_iterations; + *iterations = (int64_t *)calloc(series->num_iterations, sizeof(int64_t)); + for (int i=0; inum_iterations; i++) { + (*iterations)[i] = series->iteration_indices[i]; } - /* Free pattern info now that we're done with both branches */ +cleanup: free_iteration_pattern(&pattern_info); - /* Sort the iterations */ - if (collector.count > 0) { - qsort(collector.indices, collector.count, sizeof(int64_t), compare_int64); - } - - /* Cache results */ - series->iteration_indices = collector.indices; - series->num_iterations = collector.count; - *iterations = collector.indices; - *count = collector.count; + return status; +} - return PMD_SUCCESS; +static void pmd_invalidate_iterations_cache(pmd_series *series) { + series->num_iterations = -1; + free(series->iteration_indices); + series->iteration_indices = NULL; } /* ========================================================================= @@ -2759,19 +2775,22 @@ pmd_status pmd_open_iteration(pmd_series *series, int64_t index, pmd_iteration * char *iteration_path = NULL; if (!series || !iter_out) { - return PMD_ERROR_NULL_POINTER; + status = PMD_ERROR_NULL_POINTER; + goto cleanup; } /* Check for negative iteration index */ if (index < 0) { pmd_log(PMD_LOG_ERROR, "Iteration index must be non-negative, got %lld", index); - return PMD_ERROR; + status = PMD_ERROR; + goto cleanup; } /* Allocate iteration struct */ iter = (pmd_iteration *)calloc(1, sizeof(pmd_iteration)); if (!iter) { - return PMD_ERROR_OUT_OF_MEMORY; + status = PMD_ERROR_OUT_OF_MEMORY; + goto cleanup; } /* Initialize fields */ @@ -2817,14 +2836,7 @@ pmd_status pmd_open_iteration(pmd_series *series, int64_t index, pmd_iteration * /* Write default iteration attributes */ status = write_iteration_attributes(iter->iteration_group_id); - if (status != PMD_SUCCESS) { - goto cleanup; - } - - /* Invalidate iteration cache since we just created a new iteration */ - series->num_iterations = -1; - free(series->iteration_indices); - series->iteration_indices = NULL; + if (status != PMD_SUCCESS) goto cleanup; /* Create particles group if particlesPath is defined */ char *particles_path_copy = NULL; @@ -2976,28 +2988,15 @@ pmd_status pmd_open_iteration(pmd_series *series, int64_t index, pmd_iteration * } } - free(iteration_path); - iteration_path = NULL; - /* Register and return it */ status = register_open_iteration(series, iter); - if (status != PMD_SUCCESS) { - /* If registration fails, close what we opened and return error */ - if (iter->iteration_group_id >= 0) H5Gclose(iter->iteration_group_id); - if (series->iteration_encoding == PMD_FILE_BASED && iter->file_id >= 0) { - H5Fclose(iter->file_id); - } - free(iter); - return status; - } - - *iter_out = iter; - return PMD_SUCCESS; + if (status != PMD_SUCCESS) goto cleanup; cleanup: free(iteration_path); + iteration_path = NULL; - if (iter) { + if (iter && (status != PMD_SUCCESS)) { /* Don't close file_id for GROUP_BASED (it's owned by series) */ if (series->iteration_encoding == PMD_FILE_BASED && iter->file_id >= 0) { H5Fclose(iter->file_id); @@ -3006,8 +3005,10 @@ pmd_status pmd_open_iteration(pmd_series *series, int64_t index, pmd_iteration * H5Gclose(iter->iteration_group_id); } free(iter); + iter = NULL; } - + pmd_invalidate_iterations_cache(series); + *iter_out = iter; return status; } diff --git a/tests/test_read.c b/tests/test_read.c index 8f30855..f157652 100644 --- a/tests/test_read.c +++ b/tests/test_read.c @@ -457,7 +457,9 @@ void test_group_based_series_multiple_iterations(void) { /* Open each iteration and verify we can access it */ for (int i = 0; i < 3; i++) { pmd_iteration *iter; + printf("iteration %d: %d\n", i, iterations[i]); result = pmd_open_iteration(series, iterations[i], &iter); + printf("iteration %d: %d\n", i, iterations[i]); TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, result); TEST_ASSERT_NOT_NULL(iter); TEST_ASSERT_EQUAL_INT64(iterations[i], iter->iteration_index); From 99d5ae05e892ba472f729eb90224d587295cb7dd Mon Sep 17 00:00:00 2001 From: "Christopher M. Pierce" Date: Mon, 5 Jan 2026 23:48:54 -0500 Subject: [PATCH 12/29] more cleanup of `pmd_get_iterations` --- parcel.h | 213 +++++++++++++++++++++++++++---------------------------- 1 file changed, 104 insertions(+), 109 deletions(-) diff --git a/parcel.h b/parcel.h index 31b2d8f..ab41ea6 100644 --- a/parcel.h +++ b/parcel.h @@ -2180,142 +2180,137 @@ pmd_status pmd_get_iterations(pmd_series *series, int64_t **iterations, int *cou } series->iteration_indices[0] = 0; series->num_iterations = 1; - *iterations = series->iteration_indices; - *count = 1; - return PMD_SUCCESS; - } - - /* Parse iteration_format for both GROUP_BASED and FILE_BASED */ - pmd_status status = parse_iteration_pattern(series->iteration_format, &pattern_info); - if (status != PMD_SUCCESS) { - return status; - } + } else { + /* Parse iteration_format for both GROUP_BASED and FILE_BASED */ + status = parse_iteration_pattern(series->iteration_format, &pattern_info); + if (status != PMD_SUCCESS) goto cleanup; - /* Common collector for both GROUP_BASED and FILE_BASED */ - iteration_collector collector = {NULL, 0, 0, NULL, NULL, -1, PMD_SUCCESS}; + /* Common collector for both GROUP_BASED and FILE_BASED */ + iteration_collector collector = {NULL, 0, 0, NULL, NULL, -1, PMD_SUCCESS}; - /* Initialize collector with pattern info */ - collector.first_segment = pattern_info.first_segment; - collector.full_pattern = pattern_info.full_pattern; + /* Initialize collector with pattern info */ + collector.first_segment = pattern_info.first_segment; + collector.full_pattern = pattern_info.full_pattern; - if (series->iteration_encoding == PMD_GROUP_BASED) { + if (series->iteration_encoding == PMD_GROUP_BASED) { + /* GROUP_BASED: enumerate groups using pattern matching */ + collector.root_id = series->file_id; - /* GROUP_BASED: enumerate groups using pattern matching */ - collector.root_id = series->file_id; + /* Open parent group (scan_parent is "." for root) */ + if (record_exists(series->file_id, pattern_info.scan_parent) < 1) { + status = PMD_ERROR_FILE_FORMAT; + goto cleanup; + } + hid_t group_id = H5Gopen(series->file_id, pattern_info.scan_parent, H5P_DEFAULT); - /* Open parent group (scan_parent is "." for root) */ - if (record_exists(series->file_id, pattern_info.scan_parent) < 1) { - free_iteration_pattern(&pattern_info); - return PMD_ERROR_FILE_FORMAT; - } - hid_t group_id = H5Gopen(series->file_id, pattern_info.scan_parent, H5P_DEFAULT); + if (group_id < 0) { + status = PMD_ERROR_HDF5; + goto cleanup; + } - if (group_id < 0) { - free_iteration_pattern(&pattern_info); - return PMD_ERROR_HDF5; - } + /* Iterate through groups to find iterations */ + herr_t iter_result = H5Literate(group_id, H5_INDEX_NAME, H5_ITER_NATIVE, NULL, + collect_iterations_callback, &collector); - /* Iterate through groups to find iterations */ - herr_t iter_result = H5Literate(group_id, H5_INDEX_NAME, H5_ITER_NATIVE, NULL, - collect_iterations_callback, &collector); + H5Gclose(group_id); - H5Gclose(group_id); + /* Check if iteration callback encountered an error */ + if (iter_result < 0 && collector.status != PMD_SUCCESS) { + free(collector.indices); + status = collector.status; + goto cleanup; + } - /* Check if iteration callback encountered an error */ - if (iter_result < 0 && collector.status != PMD_SUCCESS) { - free(collector.indices); - free_iteration_pattern(&pattern_info); - return collector.status; - } + } else { /* PMD_FILE_BASED */ + /* FILE_BASED: scan directory for matching files using pattern matching */ + /* For FILE_BASED, scan_parent is always "." (validated in pmd_open_series) */ + pmd_dir *dir = pmd_opendir(series->directory); + if (!dir) { + free_iteration_pattern(&pattern_info); + return PMD_ERROR_FILE_NOT_FOUND; + } - } else { /* PMD_FILE_BASED */ + pmd_dirent *entry; - /* FILE_BASED: scan directory for matching files using pattern matching */ - /* For FILE_BASED, scan_parent is always "." (validated in pmd_open_series) */ + /* Scan directory for matching files/directories */ + while ((entry = pmd_readdir(dir)) != NULL) { + int64_t iteration; - pmd_dir *dir = pmd_opendir(series->directory); - if (!dir) { - free_iteration_pattern(&pattern_info); - return PMD_ERROR_FILE_NOT_FOUND; - } - - pmd_dirent *entry; + /* Try to extract iteration from name matching first segment pattern */ + if (extract_iteration_from_name(entry->d_name, pattern_info.first_segment, &iteration) != PMD_SUCCESS) { + continue; /* Name doesn't match, skip */ + } - /* Scan directory for matching files/directories */ - while ((entry = pmd_readdir(dir)) != NULL) { - int64_t iteration; + /* Validate full path if pattern has additional path components */ + if (pattern_info.full_pattern && (strchr(pattern_info.full_pattern, '/') || strchr(pattern_info.full_pattern, '\\'))) { + int sstatus; - /* Try to extract iteration from name matching first segment pattern */ - if (extract_iteration_from_name(entry->d_name, pattern_info.first_segment, &iteration) != PMD_SUCCESS) { - continue; /* Name doesn't match, skip */ - } + /* Build full file path with iteration substituted */ + char *rel_path = replace_iteration(series->iteration_format, iteration); + if (!rel_path) { + collector.status = PMD_ERROR_OUT_OF_MEMORY; + break; + } - /* Validate full path if pattern has additional path components */ - if (pattern_info.full_pattern && (strchr(pattern_info.full_pattern, '/') || strchr(pattern_info.full_pattern, '\\'))) { - int status; + char full_path[PMD_PATH_MAX]; + sstatus = snprintf(full_path, sizeof(full_path), "%s" PMD_PATH_SEP "%s", series->directory, rel_path); + if (sstatus < 0) { + pmd_log(PMD_LOG_ERROR, "pmd_get_iterations - failed to construct full path."); + status = PMD_ERROR; + goto cleanup; + } + free(rel_path); - /* Build full file path with iteration substituted */ - char *rel_path = replace_iteration(series->iteration_format, iteration); - if (!rel_path) { - collector.status = PMD_ERROR_OUT_OF_MEMORY; - break; + /* Check if file exists using fopen (standard C) */ + FILE *test_file = fopen(full_path, "r"); + if (!test_file) { + continue; /* Path doesn't exist, skip */ + } + sstatus = fclose(test_file); + if (sstatus < 0){ + pmd_log(PMD_LOG_ERROR, "pmd_get_iterations - failed to close test file"); + status = PMD_ERROR; + goto cleanup; + } } - char full_path[PMD_PATH_MAX]; - status = snprintf(full_path, sizeof(full_path), "%s" PMD_PATH_SEP "%s", series->directory, rel_path); - if (status < 0) { - pmd_log(PMD_LOG_ERROR, "pmd_get_iterations - failed to construct full path."); - return PMD_ERROR; + /* Grow array if needed */ + if (collector.count >= collector.capacity) { + collector.capacity = collector.capacity * 2 + 10; + int64_t *temp = (int64_t *)realloc(collector.indices, + collector.capacity * sizeof(int64_t)); + if (!temp) { + collector.status = PMD_ERROR_OUT_OF_MEMORY; + break; + } + collector.indices = temp; } - free(rel_path); - /* Check if file exists using fopen (standard C) */ - FILE *test_file = fopen(full_path, "r"); - if (!test_file) { - continue; /* Path doesn't exist, skip */ - } - status = fclose(test_file); - if (status < 0){ - pmd_log(PMD_LOG_ERROR, "pmd_get_iterations - failed to close test file"); - return PMD_ERROR; - } + collector.indices[collector.count++] = iteration; } - /* Grow array if needed */ - if (collector.count >= collector.capacity) { - collector.capacity = collector.capacity * 2 + 10; - int64_t *temp = (int64_t *)realloc(collector.indices, - collector.capacity * sizeof(int64_t)); - if (!temp) { - collector.status = PMD_ERROR_OUT_OF_MEMORY; - break; - } - collector.indices = temp; - } + pmd_closedir(dir); - collector.indices[collector.count++] = iteration; + /* Check for errors during collection */ + if (collector.status != PMD_SUCCESS) { + free(collector.indices); + free_iteration_pattern(&pattern_info); + status = collector.status; + goto cleanup; + } } - pmd_closedir(dir); - - /* Check for errors during collection */ - if (collector.status != PMD_SUCCESS) { - free(collector.indices); - free_iteration_pattern(&pattern_info); - return collector.status; + /* Sort the iterations */ + if (collector.count > 0) { + qsort(collector.indices, collector.count, sizeof(int64_t), compare_int64); } - } - /* Sort the iterations */ - if (collector.count > 0) { - qsort(collector.indices, collector.count, sizeof(int64_t), compare_int64); + /* Cache results */ + series->iteration_indices = collector.indices; + series->num_iterations = collector.count; + *iterations = collector.indices; + *count = collector.count; } - - /* Cache results */ - series->iteration_indices = collector.indices; - series->num_iterations = collector.count; - *iterations = collector.indices; - *count = collector.count; } /* Populate the user fields */ @@ -2325,9 +2320,9 @@ pmd_status pmd_get_iterations(pmd_series *series, int64_t **iterations, int *cou (*iterations)[i] = series->iteration_indices[i]; } + /* Deal with any cleanup from all execution paths (ie both failure and success) */ cleanup: free_iteration_pattern(&pattern_info); - return status; } From 19d37cd2d3593971e7b4e9ac70d207907953b0d2 Mon Sep 17 00:00:00 2001 From: "Christopher M. Pierce" Date: Tue, 6 Jan 2026 00:02:00 -0500 Subject: [PATCH 13/29] move caching/file parsing to separate spots --- parcel.h | 259 +++++++++++++++++++++++++++++-------------------------- 1 file changed, 136 insertions(+), 123 deletions(-) diff --git a/parcel.h b/parcel.h index ab41ea6..363c7b0 100644 --- a/parcel.h +++ b/parcel.h @@ -2157,160 +2157,174 @@ static herr_t collect_iterations_callback(hid_t loc_id, const char *name, return 0; /* Continue iteration */ } -pmd_status pmd_get_iterations(pmd_series *series, int64_t **iterations, int *count) { +/** + * Does the actual parsing of how many iterations and what iteration indices are available. + * `pmd_get_iterations` is the user facing function which calls this, but with caching to + * avoid hitting the disk everytime the iterations are requested. + */ +static pmd_status pmd_parse_iterations(pmd_series *series) { pmd_status status = PMD_SUCCESS; iteration_pattern pattern_info; pattern_info.scan_parent = NULL; pattern_info.first_segment = NULL; pattern_info.full_pattern = NULL; - if (!series || !iterations || !count) { - status = PMD_ERROR_NULL_POINTER; - goto cleanup; - } - - /* Fetch the iteration results if we must */ - if (series->num_iterations < 0) { - /* Check for single-snapshot file (no %T in iteration_format) */ - if (!strstr(series->iteration_format, "%T")) { - /* Single iteration at index 0 */ - series->iteration_indices = (int64_t *)malloc(sizeof(int64_t)); - if (!series->iteration_indices) { - return PMD_ERROR_OUT_OF_MEMORY; - } - series->iteration_indices[0] = 0; - series->num_iterations = 1; - } else { - /* Parse iteration_format for both GROUP_BASED and FILE_BASED */ - status = parse_iteration_pattern(series->iteration_format, &pattern_info); - if (status != PMD_SUCCESS) goto cleanup; - - /* Common collector for both GROUP_BASED and FILE_BASED */ - iteration_collector collector = {NULL, 0, 0, NULL, NULL, -1, PMD_SUCCESS}; + /* Check for single-snapshot file (no %T in iteration_format) */ + if (!strstr(series->iteration_format, "%T")) { + /* Single iteration at index 0 */ + series->iteration_indices = (int64_t *)malloc(sizeof(int64_t)); + if (!series->iteration_indices) { + return PMD_ERROR_OUT_OF_MEMORY; + } + series->iteration_indices[0] = 0; + series->num_iterations = 1; + } else { + /* Parse iteration_format for both GROUP_BASED and FILE_BASED */ + status = parse_iteration_pattern(series->iteration_format, &pattern_info); + if (status != PMD_SUCCESS) goto cleanup; - /* Initialize collector with pattern info */ - collector.first_segment = pattern_info.first_segment; - collector.full_pattern = pattern_info.full_pattern; + /* Common collector for both GROUP_BASED and FILE_BASED */ + iteration_collector collector = {NULL, 0, 0, NULL, NULL, -1, PMD_SUCCESS}; - if (series->iteration_encoding == PMD_GROUP_BASED) { - /* GROUP_BASED: enumerate groups using pattern matching */ - collector.root_id = series->file_id; + /* Initialize collector with pattern info */ + collector.first_segment = pattern_info.first_segment; + collector.full_pattern = pattern_info.full_pattern; - /* Open parent group (scan_parent is "." for root) */ - if (record_exists(series->file_id, pattern_info.scan_parent) < 1) { - status = PMD_ERROR_FILE_FORMAT; - goto cleanup; - } - hid_t group_id = H5Gopen(series->file_id, pattern_info.scan_parent, H5P_DEFAULT); + if (series->iteration_encoding == PMD_GROUP_BASED) { + /* GROUP_BASED: enumerate groups using pattern matching */ + collector.root_id = series->file_id; - if (group_id < 0) { - status = PMD_ERROR_HDF5; - goto cleanup; - } - - /* Iterate through groups to find iterations */ - herr_t iter_result = H5Literate(group_id, H5_INDEX_NAME, H5_ITER_NATIVE, NULL, - collect_iterations_callback, &collector); + /* Open parent group (scan_parent is "." for root) */ + if (record_exists(series->file_id, pattern_info.scan_parent) < 1) { + status = PMD_ERROR_FILE_FORMAT; + goto cleanup; + } + hid_t group_id = H5Gopen(series->file_id, pattern_info.scan_parent, H5P_DEFAULT); - H5Gclose(group_id); + if (group_id < 0) { + status = PMD_ERROR_HDF5; + goto cleanup; + } - /* Check if iteration callback encountered an error */ - if (iter_result < 0 && collector.status != PMD_SUCCESS) { - free(collector.indices); - status = collector.status; - goto cleanup; - } + /* Iterate through groups to find iterations */ + herr_t iter_result = H5Literate(group_id, H5_INDEX_NAME, H5_ITER_NATIVE, NULL, + collect_iterations_callback, &collector); - } else { /* PMD_FILE_BASED */ - /* FILE_BASED: scan directory for matching files using pattern matching */ - /* For FILE_BASED, scan_parent is always "." (validated in pmd_open_series) */ - pmd_dir *dir = pmd_opendir(series->directory); - if (!dir) { - free_iteration_pattern(&pattern_info); - return PMD_ERROR_FILE_NOT_FOUND; - } + H5Gclose(group_id); - pmd_dirent *entry; + /* Check if iteration callback encountered an error */ + if (iter_result < 0 && collector.status != PMD_SUCCESS) { + free(collector.indices); + status = collector.status; + goto cleanup; + } - /* Scan directory for matching files/directories */ - while ((entry = pmd_readdir(dir)) != NULL) { - int64_t iteration; + } else { /* PMD_FILE_BASED */ + /* FILE_BASED: scan directory for matching files using pattern matching */ + /* For FILE_BASED, scan_parent is always "." (validated in pmd_open_series) */ + pmd_dir *dir = pmd_opendir(series->directory); + if (!dir) { + free_iteration_pattern(&pattern_info); + return PMD_ERROR_FILE_NOT_FOUND; + } - /* Try to extract iteration from name matching first segment pattern */ - if (extract_iteration_from_name(entry->d_name, pattern_info.first_segment, &iteration) != PMD_SUCCESS) { - continue; /* Name doesn't match, skip */ - } + pmd_dirent *entry; - /* Validate full path if pattern has additional path components */ - if (pattern_info.full_pattern && (strchr(pattern_info.full_pattern, '/') || strchr(pattern_info.full_pattern, '\\'))) { - int sstatus; + /* Scan directory for matching files/directories */ + while ((entry = pmd_readdir(dir)) != NULL) { + int64_t iteration; - /* Build full file path with iteration substituted */ - char *rel_path = replace_iteration(series->iteration_format, iteration); - if (!rel_path) { - collector.status = PMD_ERROR_OUT_OF_MEMORY; - break; - } + /* Try to extract iteration from name matching first segment pattern */ + if (extract_iteration_from_name(entry->d_name, pattern_info.first_segment, &iteration) != PMD_SUCCESS) { + continue; /* Name doesn't match, skip */ + } - char full_path[PMD_PATH_MAX]; - sstatus = snprintf(full_path, sizeof(full_path), "%s" PMD_PATH_SEP "%s", series->directory, rel_path); - if (sstatus < 0) { - pmd_log(PMD_LOG_ERROR, "pmd_get_iterations - failed to construct full path."); - status = PMD_ERROR; - goto cleanup; - } - free(rel_path); + /* Validate full path if pattern has additional path components */ + if (pattern_info.full_pattern && (strchr(pattern_info.full_pattern, '/') || strchr(pattern_info.full_pattern, '\\'))) { + int sstatus; - /* Check if file exists using fopen (standard C) */ - FILE *test_file = fopen(full_path, "r"); - if (!test_file) { - continue; /* Path doesn't exist, skip */ - } - sstatus = fclose(test_file); - if (sstatus < 0){ - pmd_log(PMD_LOG_ERROR, "pmd_get_iterations - failed to close test file"); - status = PMD_ERROR; - goto cleanup; - } + /* Build full file path with iteration substituted */ + char *rel_path = replace_iteration(series->iteration_format, iteration); + if (!rel_path) { + collector.status = PMD_ERROR_OUT_OF_MEMORY; + break; } - /* Grow array if needed */ - if (collector.count >= collector.capacity) { - collector.capacity = collector.capacity * 2 + 10; - int64_t *temp = (int64_t *)realloc(collector.indices, - collector.capacity * sizeof(int64_t)); - if (!temp) { - collector.status = PMD_ERROR_OUT_OF_MEMORY; - break; - } - collector.indices = temp; + char full_path[PMD_PATH_MAX]; + sstatus = snprintf(full_path, sizeof(full_path), "%s" PMD_PATH_SEP "%s", series->directory, rel_path); + if (sstatus < 0) { + pmd_log(PMD_LOG_ERROR, "pmd_get_iterations - failed to construct full path."); + status = PMD_ERROR; + goto cleanup; } + free(rel_path); - collector.indices[collector.count++] = iteration; + /* Check if file exists using fopen (standard C) */ + FILE *test_file = fopen(full_path, "r"); + if (!test_file) { + continue; /* Path doesn't exist, skip */ + } + sstatus = fclose(test_file); + if (sstatus < 0){ + pmd_log(PMD_LOG_ERROR, "pmd_get_iterations - failed to close test file"); + status = PMD_ERROR; + goto cleanup; + } } - pmd_closedir(dir); - - /* Check for errors during collection */ - if (collector.status != PMD_SUCCESS) { - free(collector.indices); - free_iteration_pattern(&pattern_info); - status = collector.status; - goto cleanup; + /* Grow array if needed */ + if (collector.count >= collector.capacity) { + collector.capacity = collector.capacity * 2 + 10; + int64_t *temp = (int64_t *)realloc(collector.indices, + collector.capacity * sizeof(int64_t)); + if (!temp) { + collector.status = PMD_ERROR_OUT_OF_MEMORY; + break; + } + collector.indices = temp; } + + collector.indices[collector.count++] = iteration; } - /* Sort the iterations */ - if (collector.count > 0) { - qsort(collector.indices, collector.count, sizeof(int64_t), compare_int64); + pmd_closedir(dir); + + /* Check for errors during collection */ + if (collector.status != PMD_SUCCESS) { + free(collector.indices); + free_iteration_pattern(&pattern_info); + status = collector.status; + goto cleanup; } + } - /* Cache results */ - series->iteration_indices = collector.indices; - series->num_iterations = collector.count; - *iterations = collector.indices; - *count = collector.count; + /* Sort the iterations */ + if (collector.count > 0) { + qsort(collector.indices, collector.count, sizeof(int64_t), compare_int64); } + + /* Cache results */ + series->iteration_indices = collector.indices; + series->num_iterations = collector.count; + } + +cleanup: + free_iteration_pattern(&pattern_info); + return status; +} + +pmd_status pmd_get_iterations(pmd_series *series, int64_t **iterations, int *count) { + pmd_status status = PMD_SUCCESS; + + if (!series || !iterations || !count) { + status = PMD_ERROR_NULL_POINTER; + goto cleanup; + } + + /* Fetch the iteration results if we must */ + if (series->num_iterations < 0) { + status = pmd_parse_iterations(series); + if (status != PMD_SUCCESS) goto cleanup; } /* Populate the user fields */ @@ -2322,7 +2336,6 @@ pmd_status pmd_get_iterations(pmd_series *series, int64_t **iterations, int *cou /* Deal with any cleanup from all execution paths (ie both failure and success) */ cleanup: - free_iteration_pattern(&pattern_info); return status; } From a9900d4a1cf9835fc3fdae22421d61c67c997cb4 Mon Sep 17 00:00:00 2001 From: "Christopher M. Pierce" Date: Tue, 6 Jan 2026 00:15:37 -0500 Subject: [PATCH 14/29] cleaning up `pmd_open_iteration` --- parcel.h | 169 +++++++++++++++++++++++++------------------------------ 1 file changed, 77 insertions(+), 92 deletions(-) diff --git a/parcel.h b/parcel.h index 363c7b0..2c45b6a 100644 --- a/parcel.h +++ b/parcel.h @@ -2781,6 +2781,7 @@ pmd_status pmd_open_iteration(pmd_series *series, int64_t index, pmd_iteration * pmd_iteration *iter = NULL; pmd_status status = PMD_SUCCESS; char *iteration_path = NULL; + char *filename = NULL; if (!series || !iter_out) { status = PMD_ERROR_NULL_POINTER; @@ -2883,117 +2884,100 @@ pmd_status pmd_open_iteration(pmd_series *series, int64_t index, pmd_iteration * status = PMD_ERROR_HDF5; goto cleanup; } - - free(iteration_path); - iteration_path = NULL; - } else { - /* Not already open - open or create file for this iteration */ - char *filename = replace_iteration(series->iteration_format, index); - if (!filename) { - status = PMD_ERROR_OUT_OF_MEMORY; - goto cleanup; - } - char full_path[PMD_PATH_MAX]; - int s_status = snprintf(full_path, sizeof(full_path), "%s" PMD_PATH_SEP "%s", series->directory, filename); - if (s_status < 0) { - status = PMD_ERROR; - pmd_log(PMD_LOG_ERROR, "pmd_open_iteration - failed to construct full path"); - goto cleanup; - } - free(filename); - - /* Try to open existing file */ - /* Note: Convert TRUNC/EXCL to RDWR for opening existing files */ - unsigned int h5_flags; - if (series->access_mode == PMD_TRUNC || series->access_mode == PMD_EXCL) { - h5_flags = H5F_ACC_RDWR; } else { - h5_flags = pmd_access_mode_to_hdf5(series->access_mode); - } - iter->file_id = H5Fopen(full_path, h5_flags, H5P_DEFAULT); - - if (iter->file_id < 0) { - /* File doesn't exist - check if we're in write mode */ - if (series->access_mode == PMD_RDONLY) { - status = PMD_ERROR_FILE_NOT_FOUND; - goto cleanup; - } - - /* Create parent directory if needed */ - status = create_parent_directory(full_path); - if (status != PMD_SUCCESS) { + /* Not already open - open or create file for this iteration */ + filename = replace_iteration(series->iteration_format, index); + if (!filename) { + status = PMD_ERROR_OUT_OF_MEMORY; goto cleanup; } - - /* Create new file with openPMD attributes */ - iter->file_id = H5Fcreate(full_path, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT); - if (iter->file_id < 0) { - status = PMD_ERROR_HDF5; + char full_path[PMD_PATH_MAX]; + int s_status = snprintf(full_path, sizeof(full_path), "%s" PMD_PATH_SEP "%s", series->directory, filename); + if (s_status < 0) { + status = PMD_ERROR; + pmd_log(PMD_LOG_ERROR, "pmd_open_iteration - failed to construct full path"); goto cleanup; } - /* Write all root attributes to new file */ - status = write_root_attributes(iter->file_id, series); - if (status != PMD_SUCCESS) { - goto cleanup; + /* Try to open existing file */ + /* Note: Convert TRUNC/EXCL to RDWR for opening existing files */ + unsigned int h5_flags; + if (series->access_mode == PMD_TRUNC || series->access_mode == PMD_EXCL) { + h5_flags = H5F_ACC_RDWR; + } else { + h5_flags = pmd_access_mode_to_hdf5(series->access_mode); } + iter->file_id = H5Fopen(full_path, h5_flags, H5P_DEFAULT); - /* Invalidate iteration cache since we just created a new iteration file */ - series->num_iterations = -1; - free(series->iteration_indices); - series->iteration_indices = NULL; - - /* Ensure parent groups exist */ - status = ensure_parent_groups(iter->file_id, iteration_path); - if (status != PMD_SUCCESS) { - goto cleanup; - } + if (iter->file_id < 0) { + /* File doesn't exist - check if we're in write mode */ + if (series->access_mode == PMD_RDONLY) { + status = PMD_ERROR_FILE_NOT_FOUND; + goto cleanup; + } - /* Create iteration group */ - iter->iteration_group_id = H5Gcreate(iter->file_id, iteration_path, - H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT); - if (iter->iteration_group_id < 0) { - status = PMD_ERROR_HDF5; - goto cleanup; - } + /* Create parent directory if needed */ + status = create_parent_directory(full_path); + if (status != PMD_SUCCESS) goto cleanup; - /* Write iteration attributes */ - status = write_iteration_attributes(iter->iteration_group_id); - if (status != PMD_SUCCESS) { - goto cleanup; - } + /* Create new file with openPMD attributes */ + iter->file_id = H5Fcreate(full_path, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT); + if (iter->file_id < 0) { + status = PMD_ERROR_HDF5; + goto cleanup; + } - /* Create particles group if particlesPath is defined */ - char *particles_path_copy = NULL; - status = pmd_get_particles_path(series, &particles_path_copy); - if (status == PMD_SUCCESS) { - /* particlesPath is defined, create the group */ + /* Write all root attributes to new file */ + status = write_root_attributes(iter->file_id, series); + if (status != PMD_SUCCESS) goto cleanup; - /* Remove trailing slash */ - size_t len = strlen(particles_path_copy); - if (len > 0 && particles_path_copy[len-1] == '/') { - particles_path_copy[len-1] = '\0'; - } + /* Ensure parent groups exist */ + status = ensure_parent_groups(iter->file_id, iteration_path); + if (status != PMD_SUCCESS) goto cleanup; - hid_t particles_group = H5Gcreate(iter->iteration_group_id, particles_path_copy, + /* Create iteration group */ + iter->iteration_group_id = H5Gcreate(iter->file_id, iteration_path, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT); - free(particles_path_copy); - if (particles_group < 0) { + if (iter->iteration_group_id < 0) { status = PMD_ERROR_HDF5; goto cleanup; } - H5Gclose(particles_group); - } - /* If particlesPath not defined, that's OK - file has no particles */ - } else { - /* File exists - open iteration group */ - iter->iteration_group_id = H5Gopen(iter->file_id, iteration_path, H5P_DEFAULT); - if (iter->iteration_group_id < 0) { - status = PMD_ERROR_INVALID_ITERATION; - goto cleanup; + + /* Write iteration attributes */ + status = write_iteration_attributes(iter->iteration_group_id); + if (status != PMD_SUCCESS) goto cleanup; + + /* Create particles group if particlesPath is defined */ + char *particles_path_copy = NULL; + status = pmd_get_particles_path(series, &particles_path_copy); + if (status == PMD_SUCCESS) { + /* particlesPath is defined, create the group */ + + /* Remove trailing slash */ + size_t len = strlen(particles_path_copy); + if (len > 0 && particles_path_copy[len-1] == '/') { + particles_path_copy[len-1] = '\0'; + } + + hid_t particles_group = H5Gcreate(iter->iteration_group_id, particles_path_copy, + H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT); + free(particles_path_copy); + if (particles_group < 0) { + status = PMD_ERROR_HDF5; + goto cleanup; + } + H5Gclose(particles_group); + } + /* If particlesPath not defined, that's OK - file has no particles */ + } else { + /* File exists - open iteration group */ + iter->iteration_group_id = H5Gopen(iter->file_id, iteration_path, H5P_DEFAULT); + if (iter->iteration_group_id < 0) { + status = PMD_ERROR_INVALID_ITERATION; + goto cleanup; + } } } - } } /* Register and return it */ @@ -3001,6 +2985,7 @@ pmd_status pmd_open_iteration(pmd_series *series, int64_t index, pmd_iteration * if (status != PMD_SUCCESS) goto cleanup; cleanup: + free(filename); free(iteration_path); iteration_path = NULL; From 93e807b34c361234ea7cc7feb66ed48708ff9427 Mon Sep 17 00:00:00 2001 From: "Christopher M. Pierce" Date: Tue, 6 Jan 2026 00:34:44 -0500 Subject: [PATCH 15/29] resolving memory leaks --- parcel.h | 14 ++++++++++---- tests/test_write.c | 19 +++++++++++++++++++ 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/parcel.h b/parcel.h index 2c45b6a..e6ad002 100644 --- a/parcel.h +++ b/parcel.h @@ -2042,6 +2042,7 @@ pmd_status pmd_open_series(const char *filename, pmd_series **series_out, pmd_ac pmd_close_series(series); series = NULL; } + free(full_path); free(actual_filename); free_iteration_pattern(&pattern_info); *series_out = series; @@ -3332,7 +3333,7 @@ pmd_status pmd_get_openpmd_version(pmd_series *series, int *major, int *minor, i file_id = series->file_id; } else { /* FILE_BASED: need to open first iteration to get to file */ - int64_t *iterations; + int64_t *iterations = NULL; int num_iterations; status = pmd_get_iterations(series, &iterations, &num_iterations); if (status != PMD_SUCCESS) { @@ -3392,7 +3393,7 @@ static pmd_status read_series_root_attribute(pmd_series *series, const char *att } else { /* FILE_BASED: need to open one of the files to read root attributes */ /* Get first available iteration */ - int64_t *iterations; + int64_t *iterations = NULL; int num_iterations; status = pmd_get_iterations(series, &iterations, &num_iterations); if (status != PMD_SUCCESS || num_iterations == 0) { @@ -3405,6 +3406,7 @@ static pmd_status read_series_root_attribute(pmd_series *series, const char *att return status; } file_id = iter->file_id; + free(iterations); } /* Check if attribute exists */ @@ -3563,7 +3565,7 @@ static pmd_status write_series_root_attribute(pmd_series *series, const char *at status = write_string_attribute(file_id, attr_name, value); } else { /* FILE_BASED: need to write to all iteration files */ - int64_t *iterations; + int64_t *iterations = NULL; int num_iterations; status = pmd_get_iterations(series, &iterations, &num_iterations); if (status != PMD_SUCCESS) { @@ -3584,6 +3586,8 @@ static pmd_status write_series_root_attribute(pmd_series *series, const char *at return status; } } + + free(iterations); } return PMD_SUCCESS; @@ -3611,7 +3615,7 @@ static pmd_status write_series_root_attributes(pmd_series *series) { status = write_root_attributes(series->file_id, series); } else { /* FILE_BASED: write to all iteration files */ - int64_t *iterations; + int64_t *iterations = NULL; int num_iterations; status = pmd_get_iterations(series, &iterations, &num_iterations); if (status != PMD_SUCCESS) { @@ -3639,6 +3643,8 @@ static pmd_status write_series_root_attributes(pmd_series *series) { return status; } } + + free(iterations); } return PMD_SUCCESS; diff --git a/tests/test_write.c b/tests/test_write.c index 28699f4..7f666fc 100644 --- a/tests/test_write.c +++ b/tests/test_write.c @@ -598,6 +598,7 @@ void test_get_iterations_cache_group_based(void) { result = pmd_get_iterations(series, &iterations, &num_iterations); TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, result); TEST_ASSERT_EQUAL_INT(0, num_iterations); + free(iterations); /* Create first iteration */ result = pmd_open_iteration(series, 0, &iter); @@ -610,6 +611,7 @@ void test_get_iterations_cache_group_based(void) { TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, result); TEST_ASSERT_EQUAL_INT(1, num_iterations); TEST_ASSERT_EQUAL_INT64(0, iterations[0]); + free(iterations); /* Create second iteration */ result = pmd_open_iteration(series, 1, &iter); @@ -623,6 +625,7 @@ void test_get_iterations_cache_group_based(void) { TEST_ASSERT_EQUAL_INT(2, num_iterations); TEST_ASSERT_EQUAL_INT64(0, iterations[0]); TEST_ASSERT_EQUAL_INT64(1, iterations[1]); + free(iterations); /* Create third iteration */ result = pmd_open_iteration(series, 2, &iter); @@ -637,6 +640,7 @@ void test_get_iterations_cache_group_based(void) { TEST_ASSERT_EQUAL_INT64(0, iterations[0]); TEST_ASSERT_EQUAL_INT64(1, iterations[1]); TEST_ASSERT_EQUAL_INT64(2, iterations[2]); + free(iterations); result = pmd_close_series(series); TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, result); @@ -660,6 +664,7 @@ void test_get_iterations_cache_file_based(void) { result = pmd_get_iterations(series, &iterations, &num_iterations); TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, result); TEST_ASSERT_EQUAL_INT(0, num_iterations); + free(iterations); /* Create first iteration */ result = pmd_open_iteration(series, 0, &iter); @@ -672,6 +677,7 @@ void test_get_iterations_cache_file_based(void) { TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, result); TEST_ASSERT_EQUAL_INT(1, num_iterations); TEST_ASSERT_EQUAL_INT64(0, iterations[0]); + free(iterations); /* Create second iteration */ result = pmd_open_iteration(series, 1, &iter); @@ -685,6 +691,7 @@ void test_get_iterations_cache_file_based(void) { TEST_ASSERT_EQUAL_INT(2, num_iterations); TEST_ASSERT_EQUAL_INT64(0, iterations[0]); TEST_ASSERT_EQUAL_INT64(1, iterations[1]); + free(iterations); /* Create third iteration */ result = pmd_open_iteration(series, 2, &iter); @@ -699,6 +706,7 @@ void test_get_iterations_cache_file_based(void) { TEST_ASSERT_EQUAL_INT64(0, iterations[0]); TEST_ASSERT_EQUAL_INT64(1, iterations[1]); TEST_ASSERT_EQUAL_INT64(2, iterations[2]); + free(iterations); result = pmd_close_series(series); TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, result); @@ -729,6 +737,7 @@ void test_get_iterations_nonconsecutive_group_based(void) { TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, result); TEST_ASSERT_EQUAL_INT(1, num_iterations); TEST_ASSERT_EQUAL_INT64(0, iterations[0]); + free(iterations); result = pmd_open_iteration(series, 5, &iter); TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, result); @@ -741,6 +750,7 @@ void test_get_iterations_nonconsecutive_group_based(void) { TEST_ASSERT_EQUAL_INT(2, num_iterations); TEST_ASSERT_EQUAL_INT64(0, iterations[0]); TEST_ASSERT_EQUAL_INT64(5, iterations[1]); + free(iterations); result = pmd_open_iteration(series, 10, &iter); TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, result); @@ -754,6 +764,7 @@ void test_get_iterations_nonconsecutive_group_based(void) { TEST_ASSERT_EQUAL_INT64(0, iterations[0]); TEST_ASSERT_EQUAL_INT64(5, iterations[1]); TEST_ASSERT_EQUAL_INT64(10, iterations[2]); + free(iterations); result = pmd_close_series(series); TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, result); @@ -789,6 +800,7 @@ void test_get_iterations_nonconsecutive_file_based(void) { TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, result); result = pmd_close_iteration(iter); TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, result); + free(iterations); /* Check after second iteration */ result = pmd_get_iterations(series, &iterations, &num_iterations); @@ -801,6 +813,7 @@ void test_get_iterations_nonconsecutive_file_based(void) { TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, result); result = pmd_close_iteration(iter); TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, result); + free(iterations); /* Check after third iteration */ result = pmd_get_iterations(series, &iterations, &num_iterations); @@ -812,6 +825,7 @@ void test_get_iterations_nonconsecutive_file_based(void) { result = pmd_close_series(series); TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, result); + free(iterations); } /** @@ -896,6 +910,7 @@ void test_write_nonconsecutive_iterations_group_based(void) { result = pmd_close_series(series); TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, result); + free(iterations); } /** @@ -954,6 +969,7 @@ void test_write_nonconsecutive_iterations_file_based(void) { result = pmd_close_series(series); TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, result); + free(iterations); } /** @@ -1251,6 +1267,7 @@ void test_openpmd_required_attributes(void) { free(px); free(py); free(pz); + free(iterations); } } @@ -2047,6 +2064,7 @@ void test_trunc_deletes_existing_file_based(void) { if (result == PMD_SUCCESS) { TEST_ASSERT_EQUAL_INT_MESSAGE(0, num_iterations, "Should have no iterations after TRUNC"); } + free(iterations); /* Verify old files were deleted */ f0 = fopen(TEST_TEMP_DIR "/trunc_test_0.h5", "rb"); @@ -2075,6 +2093,7 @@ void test_trunc_deletes_existing_file_based(void) { TEST_ASSERT_EQUAL_INT(2, num_iterations); TEST_ASSERT_EQUAL_INT64(5, iterations[0]); TEST_ASSERT_EQUAL_INT64(10, iterations[1]); + free(iterations); result = pmd_close_series(series); TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, result); From b607fa7f6ff80c27db9ea9a28cf1dfb8667f2956 Mon Sep 17 00:00:00 2001 From: "Christopher M. Pierce" Date: Tue, 6 Jan 2026 00:41:27 -0500 Subject: [PATCH 16/29] more valgrind --- parcel.h | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/parcel.h b/parcel.h index e6ad002..de68a07 100644 --- a/parcel.h +++ b/parcel.h @@ -1704,7 +1704,6 @@ pmd_status pmd_open_series(const char *filename, pmd_series **series_out, pmd_ac hid_t file_id = -1; pmd_status status = PMD_SUCCESS; char *actual_filename = NULL; - char *full_path = NULL; int is_write_mode = (mode != PMD_RDONLY); int file_exists = 0; @@ -1776,18 +1775,17 @@ pmd_status pmd_open_series(const char *filename, pmd_series **series_out, pmd_ac if (extract_iteration_from_name(entry->d_name, pattern_info.first_segment, &iteration) == PMD_SUCCESS) { /* Reconstruct full file path from pattern. We are allocating memory in a loop, free here if */ /* memory was allocated before allocating more memory to avoid leak. */ - free(full_path); - full_path = replace_iteration(filename, iteration); - if (!full_path) { + free(actual_filename); + actual_filename = replace_iteration(filename, iteration); + if (!actual_filename) { pmd_closedir(dir); status = PMD_ERROR_OUT_OF_MEMORY; goto cleanup; } // Open the HDF5 file - file_id = H5Fopen(full_path, H5F_ACC_RDONLY, H5P_DEFAULT); + file_id = H5Fopen(actual_filename, H5F_ACC_RDONLY, H5P_DEFAULT); if (file_id >= 0) { - actual_filename = full_path; found = 1; /* Read metadata from the opened file */ @@ -2042,7 +2040,6 @@ pmd_status pmd_open_series(const char *filename, pmd_series **series_out, pmd_ac pmd_close_series(series); series = NULL; } - free(full_path); free(actual_filename); free_iteration_pattern(&pattern_info); *series_out = series; From 5bee5255c61832393c28a213205c67a7fa5ccecd Mon Sep 17 00:00:00 2001 From: "Christopher M. Pierce" Date: Tue, 6 Jan 2026 12:46:50 -0500 Subject: [PATCH 17/29] fixing memory leaks --- parcel.h | 73 ++++++++++++++++++++-------------------------- tests/test_write.c | 2 +- 2 files changed, 33 insertions(+), 42 deletions(-) diff --git a/parcel.h b/parcel.h index de68a07..533b463 100644 --- a/parcel.h +++ b/parcel.h @@ -1703,7 +1703,6 @@ pmd_status pmd_open_series(const char *filename, pmd_series **series_out, pmd_ac pmd_series *series = NULL; hid_t file_id = -1; pmd_status status = PMD_SUCCESS; - char *actual_filename = NULL; int is_write_mode = (mode != PMD_RDONLY); int file_exists = 0; @@ -1767,37 +1766,14 @@ pmd_status pmd_open_series(const char *filename, pmd_series **series_out, pmd_ac /* Find first file matching the pattern */ int found = 0; + int64_t first_iteration; pmd_dirent *entry; if (dir) { while ((entry = pmd_readdir(dir)) != NULL && !found) { - int64_t iteration; /* Try to extract iteration from name matching first segment pattern */ - if (extract_iteration_from_name(entry->d_name, pattern_info.first_segment, &iteration) == PMD_SUCCESS) { - /* Reconstruct full file path from pattern. We are allocating memory in a loop, free here if */ - /* memory was allocated before allocating more memory to avoid leak. */ - free(actual_filename); - actual_filename = replace_iteration(filename, iteration); - if (!actual_filename) { - pmd_closedir(dir); - status = PMD_ERROR_OUT_OF_MEMORY; - goto cleanup; - } - - // Open the HDF5 file - file_id = H5Fopen(actual_filename, H5F_ACC_RDONLY, H5P_DEFAULT); - if (file_id >= 0) { - found = 1; - - /* Read metadata from the opened file */ - status = read_series_metadata_from_file(file_id, series, filename); - if (status != PMD_SUCCESS) { - goto cleanup; - } - - /* Close file since we will not store for file-based mode */ - H5Fclose(file_id); - file_id = -1; - } + if (extract_iteration_from_name(entry->d_name, pattern_info.first_segment, &first_iteration) == PMD_SUCCESS) { + found = 1; + break; } } pmd_closedir(dir); @@ -1854,7 +1830,7 @@ pmd_status pmd_open_series(const char *filename, pmd_series **series_out, pmd_ac int sstatus = snprintf(del_path, sizeof(del_path), "%s" PMD_PATH_SEP "%s", pattern_info.scan_parent, del_entry->d_name); if (sstatus < 0) { - pmd_log(PMD_LOG_ERROR, "pmd_open_series - failed to construct path of file to delete in truncate mode."); + pmd_log(PMD_LOG_ERROR, "pmd_open_series - failed to construct path of file to delete in truncate mode.\n"); status = PMD_ERROR; goto cleanup; } @@ -1862,7 +1838,7 @@ pmd_status pmd_open_series(const char *filename, pmd_series **series_out, pmd_ac // Delete the file int rstatus = remove(del_path); if (rstatus < 0) { - pmd_log(PMD_LOG_ERROR, "pmd_open_series - failed to delete existing iterations in truncate mode."); + pmd_log(PMD_LOG_ERROR, "pmd_open_series - failed to delete existing iterations in truncate mode.\n"); status = PMD_ERROR; goto cleanup; } @@ -1871,10 +1847,6 @@ pmd_status pmd_open_series(const char *filename, pmd_series **series_out, pmd_ac pmd_closedir(del_dir); } - /* Set actual_filename to NULL since we deleted the files */ - free(actual_filename); - actual_filename = NULL; - /* Set up series for FILE_BASED mode (files will be created on demand) */ series->iteration_encoding = PMD_FILE_BASED; @@ -1897,6 +1869,27 @@ pmd_status pmd_open_series(const char *filename, pmd_series **series_out, pmd_ac /* Don't create any files yet - will be created when iterations are added */ series->file_id = -1; + } else { + char *iter_filename = replace_iteration(filename, first_iteration); + if (!iter_filename) { + pmd_closedir(dir); + status = PMD_ERROR_OUT_OF_MEMORY; + goto cleanup; + } + + // Open the HDF5 file + file_id = H5Fopen(iter_filename, H5F_ACC_RDONLY, H5P_DEFAULT); + if (file_id >= 0) { + /* Read metadata from the opened file */ + status = read_series_metadata_from_file(file_id, series, filename); + if (status != PMD_SUCCESS) { + goto cleanup; + } + + /* Close file since we will not store for file-based mode */ + H5Fclose(file_id); + file_id = -1; + } } /* Set directory from pattern if not already set */ @@ -1980,14 +1973,13 @@ pmd_status pmd_open_series(const char *filename, pmd_series **series_out, pmd_ac status = PMD_ERROR_HDF5; goto cleanup; } - actual_filename = strdup(filename); series->file_id = file_id; - } - /* Read metadata from the opened file */ - status = read_series_metadata_from_file(file_id, series, filename); - if (status != PMD_SUCCESS) { - goto cleanup; + /* Read metadata from the opened file */ + status = read_series_metadata_from_file(file_id, series, filename); + if (status != PMD_SUCCESS) { + goto cleanup; + } } /* For FILE_BASED series, set directory to parent of filename */ @@ -2040,7 +2032,6 @@ pmd_status pmd_open_series(const char *filename, pmd_series **series_out, pmd_ac pmd_close_series(series); series = NULL; } - free(actual_filename); free_iteration_pattern(&pattern_info); *series_out = series; return status; diff --git a/tests/test_write.c b/tests/test_write.c index 7f666fc..3f8597c 100644 --- a/tests/test_write.c +++ b/tests/test_write.c @@ -2445,7 +2445,7 @@ void test_windows_path_rdwr(void) { int main(void) { /* Suppress error messages during tests */ H5Eset_auto2(H5E_DEFAULT, NULL, NULL); - pmd_set_log_level(PMD_LOG_NONE); + //pmd_set_log_level(PMD_LOG_NONE); UNITY_BEGIN(); From c1af73bd8734e3639e335dfac7e6be9fb1906061 Mon Sep 17 00:00:00 2001 From: "Christopher M. Pierce" Date: Tue, 6 Jan 2026 12:57:54 -0500 Subject: [PATCH 18/29] typo --- tests/test_write.c | 128 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 128 insertions(+) diff --git a/tests/test_write.c b/tests/test_write.c index 3f8597c..27f4045 100644 --- a/tests/test_write.c +++ b/tests/test_write.c @@ -11,6 +11,7 @@ #include #include #include +#include #ifdef _WIN32 #include @@ -1090,6 +1091,132 @@ void test_valid_filebased_patterns(void) { } } +/** + * Test: Verify truncate mode deletes existing iteration files + * Tests that PMD_TRUNC properly removes old files for various patterns + */ +void test_truncate_deletes_existing_files(void) { + typedef struct { + const char *pattern; + int64_t iterations[3]; /* Test iterations to create */ + const char *description; + } truncate_test_case; + + truncate_test_case test_cases[] = { + { + TEST_TEMP_DIR "/trunc_simple_%T.h5", + {10, 20, 30}, + "Simple pattern" + }, + { + TEST_TEMP_DIR "/trunc_dir_%T/data.h5", + {10, 20, 30}, + "Directory with %T" + }, + { + TEST_TEMP_DIR "/trunc_multi_%T_%T.h5", + {10, 20, 30}, + "Multiple %T in filename" + }, + { + TEST_TEMP_DIR "/trunc_nested_%T/step_%T/data.h5", + {10, 20, 30}, + "Nested directories with %T" + }, + { + TEST_TEMP_DIR "/trunc_complex_%T/file_%T.h5", + {10, 20, 30}, + "Complex pattern with directory and file %T" + } + }; + + int num_cases = sizeof(test_cases) / sizeof(test_cases[0]); + + for (int i = 0; i < num_cases; i++) { + pmd_series *series; + pmd_iteration *iter; + pmd_status result; + + /* Step 1: Create series with some iterations */ + result = pmd_open_series(test_cases[i].pattern, &series, PMD_TRUNC); + TEST_ASSERT_EQUAL_INT_MESSAGE(PMD_SUCCESS, result, test_cases[i].description); + + for (int j = 0; j < 3; j++) { + result = pmd_open_iteration(series, test_cases[i].iterations[j], &iter); + TEST_ASSERT_EQUAL_INT_MESSAGE(PMD_SUCCESS, result, test_cases[i].description); + result = pmd_close_iteration(iter); + TEST_ASSERT_EQUAL_INT_MESSAGE(PMD_SUCCESS, result, test_cases[i].description); + } + + result = pmd_close_series(series); + TEST_ASSERT_EQUAL_INT_MESSAGE(PMD_SUCCESS, result, test_cases[i].description); + + /* Step 2: Verify files exist */ + for (int j = 0; j < 3; j++) { + char filepath[512]; + const char *pattern = test_cases[i].pattern; + const char *p = pattern; + char *out = filepath; + char iter_str[32]; + (void)snprintf(iter_str, sizeof(iter_str), "%" PRId64, test_cases[i].iterations[j]); + + /* Replace all %T with iteration number */ + while (*p && (out - filepath) < (int)sizeof(filepath) - 1) { + if (p[0] == '%' && p[1] == 'T') { + strcpy(out, iter_str); + out += strlen(iter_str); + p += 2; + } else { + *out++ = *p++; + } + } + *out = '\0'; + + FILE *test = fopen(filepath, "rb"); + char msg[256]; + (void)snprintf(msg, sizeof(msg), "%s - file should exist before truncate: %s", + test_cases[i].description, filepath); + TEST_ASSERT_NOT_NULL_MESSAGE(test, msg); + if (test) (void)fclose(test); + } + + /* Step 3: Reopen in truncate mode */ + result = pmd_open_series(test_cases[i].pattern, &series, PMD_TRUNC); + TEST_ASSERT_EQUAL_INT_MESSAGE(PMD_SUCCESS, result, test_cases[i].description); + result = pmd_close_series(series); + TEST_ASSERT_EQUAL_INT_MESSAGE(PMD_SUCCESS, result, test_cases[i].description); + + /* Step 4: Verify old files were deleted */ + for (int j = 0; j < 3; j++) { + char filepath[512]; + const char *pattern = test_cases[i].pattern; + const char *p = pattern; + char *out = filepath; + char iter_str[32]; + (void)snprintf(iter_str, sizeof(iter_str), "%" PRId64, test_cases[i].iterations[j]); + + /* Replace all %T with iteration number */ + while (*p && (out - filepath) < (int)sizeof(filepath) - 1) { + if (p[0] == '%' && p[1] == 'T') { + strcpy(out, iter_str); + out += strlen(iter_str); + p += 2; + } else { + *out++ = *p++; + } + } + *out = '\0'; + + FILE *test = fopen(filepath, "rb"); + char msg[256]; + (void)snprintf(msg, sizeof(msg), "%s - file should be deleted after truncate: %s", + test_cases[i].description, filepath); + TEST_ASSERT_NULL_MESSAGE(test, msg); + if (test) (void)fclose(test); + } + } +} + /** * Test: Verify all required OpenPMD attributes are written * Parameterized test for both file-based and group-based series @@ -2469,6 +2596,7 @@ int main(void) { RUN_TEST(test_write_fails_no_parent_directory); RUN_TEST(test_invalid_pattern_ambiguous); RUN_TEST(test_valid_filebased_patterns); + RUN_TEST(test_truncate_deletes_existing_files); RUN_TEST(test_filebased_fails_parent_before_t_missing); RUN_TEST(test_openpmd_required_attributes); RUN_TEST(test_write_particle_group_minimal); From ee51e843e906ff763adf362f706dab14ab827630 Mon Sep 17 00:00:00 2001 From: "Christopher M. Pierce" Date: Tue, 6 Jan 2026 13:04:14 -0500 Subject: [PATCH 19/29] fix directory detection issue --- parcel.h | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/parcel.h b/parcel.h index 533b463..e61d414 100644 --- a/parcel.h +++ b/parcel.h @@ -1772,8 +1772,25 @@ pmd_status pmd_open_series(const char *filename, pmd_series **series_out, pmd_ac while ((entry = pmd_readdir(dir)) != NULL && !found) { /* Try to extract iteration from name matching first segment pattern */ if (extract_iteration_from_name(entry->d_name, pattern_info.first_segment, &first_iteration) == PMD_SUCCESS) { - found = 1; - break; + /* Reconstruct full file path from pattern (ie for paths that look like data_%T/file_%T.h5 where we only found + * the directory data_1) */ + char *full_path = replace_iteration(filename, first_iteration); + if (!full_path) { + pmd_closedir(dir); + free_iteration_pattern(&pattern_info); + free(series); + return PMD_ERROR_OUT_OF_MEMORY; + } + + /* Test for file existance */ + FILE *test = fopen(full_path, "rb"); + if (test) { + found = 1; + free(full_path); + (void)fclose(test); + break; + } + free(full_path); } } pmd_closedir(dir); From 3b3735067a7af37ec16519edc390a95d126f3e63 Mon Sep 17 00:00:00 2001 From: "Christopher M. Pierce" Date: Tue, 6 Jan 2026 13:10:07 -0500 Subject: [PATCH 20/29] move deletion code to new function --- parcel.h | 74 +++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 47 insertions(+), 27 deletions(-) diff --git a/parcel.h b/parcel.h index e61d414..da8963e 100644 --- a/parcel.h +++ b/parcel.h @@ -1699,6 +1699,49 @@ static pmd_status read_series_metadata_from_file(hid_t file_id, pmd_series *seri return PMD_SUCCESS; } +/** + * Delete all iteration files matching the pattern + * Used during truncate mode to remove existing iterations + */ +static pmd_status delete_matching_iteration_files(const char *pattern) { + iteration_pattern pattern_info; + if (parse_iteration_pattern(pattern, &pattern_info) != PMD_SUCCESS) { + return PMD_ERROR; + } + + pmd_dir *del_dir = pmd_opendir(pattern_info.scan_parent); + if (!del_dir) { + /* Directory doesn't exist - nothing to delete */ + return PMD_SUCCESS; + } + + pmd_dirent *del_entry; + while ((del_entry = pmd_readdir(del_dir)) != NULL) { + int64_t iter_index; + if (extract_iteration_from_name(del_entry->d_name, pattern_info.first_segment, &iter_index) == PMD_SUCCESS) { + /* Construct path to delete */ + char del_path[PMD_PATH_MAX]; + int sstatus = snprintf(del_path, sizeof(del_path), "%s" PMD_PATH_SEP "%s", + pattern_info.scan_parent, del_entry->d_name); + if (sstatus < 0 || sstatus >= PMD_PATH_MAX) { + pmd_closedir(del_dir); + pmd_log(PMD_LOG_ERROR, "delete_matching_iteration_files - failed to construct path of file to delete.\n"); + return PMD_ERROR; + } + + /* Delete the file or directory */ + int rstatus = remove(del_path); + if (rstatus != 0) { + pmd_closedir(del_dir); + pmd_log(PMD_LOG_ERROR, "delete_matching_iteration_files - failed to delete: %s\n", del_path); + return PMD_ERROR; + } + } + } + pmd_closedir(del_dir); + return PMD_SUCCESS; +} + pmd_status pmd_open_series(const char *filename, pmd_series **series_out, pmd_access_mode mode) { pmd_series *series = NULL; hid_t file_id = -1; @@ -1835,33 +1878,10 @@ pmd_status pmd_open_series(const char *filename, pmd_series **series_out, pmd_ac file_id = -1; } - /* Enumerate and delete all matching iteration files */ - pmd_dir *del_dir = pmd_opendir(pattern_info.scan_parent); - if (del_dir) { - pmd_dirent *del_entry; - while ((del_entry = pmd_readdir(del_dir)) != NULL) { - int64_t iter_index; - if (extract_iteration_from_name(del_entry->d_name, pattern_info.first_segment, &iter_index) == PMD_SUCCESS) { - // Construct path to delete - char del_path[PMD_PATH_MAX]; - int sstatus = snprintf(del_path, sizeof(del_path), "%s" PMD_PATH_SEP "%s", - pattern_info.scan_parent, del_entry->d_name); - if (sstatus < 0) { - pmd_log(PMD_LOG_ERROR, "pmd_open_series - failed to construct path of file to delete in truncate mode.\n"); - status = PMD_ERROR; - goto cleanup; - } - - // Delete the file - int rstatus = remove(del_path); - if (rstatus < 0) { - pmd_log(PMD_LOG_ERROR, "pmd_open_series - failed to delete existing iterations in truncate mode.\n"); - status = PMD_ERROR; - goto cleanup; - } - } - } - pmd_closedir(del_dir); + /* Delete all matching iteration files */ + status = delete_matching_iteration_files(filename); + if (status != PMD_SUCCESS) { + goto cleanup; } /* Set up series for FILE_BASED mode (files will be created on demand) */ From 70914c4f7e22e24a338a342eb189c029823e8563 Mon Sep 17 00:00:00 2001 From: "Christopher M. Pierce" Date: Tue, 6 Jan 2026 13:14:50 -0500 Subject: [PATCH 21/29] cleaning up --- parcel.h | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/parcel.h b/parcel.h index da8963e..36a77f5 100644 --- a/parcel.h +++ b/parcel.h @@ -1704,6 +1704,7 @@ static pmd_status read_series_metadata_from_file(hid_t file_id, pmd_series *seri * Used during truncate mode to remove existing iterations */ static pmd_status delete_matching_iteration_files(const char *pattern) { + pmd_status status = PMD_SUCCESS; iteration_pattern pattern_info; if (parse_iteration_pattern(pattern, &pattern_info) != PMD_SUCCESS) { return PMD_ERROR; @@ -1712,6 +1713,7 @@ static pmd_status delete_matching_iteration_files(const char *pattern) { pmd_dir *del_dir = pmd_opendir(pattern_info.scan_parent); if (!del_dir) { /* Directory doesn't exist - nothing to delete */ + free_iteration_pattern(&pattern_info); return PMD_SUCCESS; } @@ -1719,26 +1721,28 @@ static pmd_status delete_matching_iteration_files(const char *pattern) { while ((del_entry = pmd_readdir(del_dir)) != NULL) { int64_t iter_index; if (extract_iteration_from_name(del_entry->d_name, pattern_info.first_segment, &iter_index) == PMD_SUCCESS) { - /* Construct path to delete */ char del_path[PMD_PATH_MAX]; int sstatus = snprintf(del_path, sizeof(del_path), "%s" PMD_PATH_SEP "%s", pattern_info.scan_parent, del_entry->d_name); if (sstatus < 0 || sstatus >= PMD_PATH_MAX) { - pmd_closedir(del_dir); pmd_log(PMD_LOG_ERROR, "delete_matching_iteration_files - failed to construct path of file to delete.\n"); - return PMD_ERROR; + status = PMD_ERROR; + goto cleanup; } /* Delete the file or directory */ int rstatus = remove(del_path); if (rstatus != 0) { - pmd_closedir(del_dir); pmd_log(PMD_LOG_ERROR, "delete_matching_iteration_files - failed to delete: %s\n", del_path); - return PMD_ERROR; + status = PMD_ERROR; + goto cleanup; } } } + +cleanup: pmd_closedir(del_dir); + free_iteration_pattern(&pattern_info); return PMD_SUCCESS; } From 548cf3cd5e3a7d5562bcb114769771c24bc43932 Mon Sep 17 00:00:00 2001 From: "Christopher M. Pierce" Date: Tue, 6 Jan 2026 13:17:48 -0500 Subject: [PATCH 22/29] correctly delete full path to iteration file --- parcel.h | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/parcel.h b/parcel.h index 36a77f5..b0a8f12 100644 --- a/parcel.h +++ b/parcel.h @@ -1706,6 +1706,8 @@ static pmd_status read_series_metadata_from_file(hid_t file_id, pmd_series *seri static pmd_status delete_matching_iteration_files(const char *pattern) { pmd_status status = PMD_SUCCESS; iteration_pattern pattern_info; + char *full_path = NULL; + if (parse_iteration_pattern(pattern, &pattern_info) != PMD_SUCCESS) { return PMD_ERROR; } @@ -1721,19 +1723,19 @@ static pmd_status delete_matching_iteration_files(const char *pattern) { while ((del_entry = pmd_readdir(del_dir)) != NULL) { int64_t iter_index; if (extract_iteration_from_name(del_entry->d_name, pattern_info.first_segment, &iter_index) == PMD_SUCCESS) { - char del_path[PMD_PATH_MAX]; - int sstatus = snprintf(del_path, sizeof(del_path), "%s" PMD_PATH_SEP "%s", - pattern_info.scan_parent, del_entry->d_name); - if (sstatus < 0 || sstatus >= PMD_PATH_MAX) { - pmd_log(PMD_LOG_ERROR, "delete_matching_iteration_files - failed to construct path of file to delete.\n"); - status = PMD_ERROR; + /* Reconstruct full file path from pattern (for paths like data_%T/file_%T.h5 + * where we only found the directory data_1) */ + full_path = replace_iteration(pattern, iter_index); + if (!full_path) { + pmd_log(PMD_LOG_ERROR, "delete_matching_iteration_files - out of memory constructing path.\n"); + status = PMD_ERROR_OUT_OF_MEMORY; goto cleanup; } /* Delete the file or directory */ - int rstatus = remove(del_path); + int rstatus = remove(full_path); if (rstatus != 0) { - pmd_log(PMD_LOG_ERROR, "delete_matching_iteration_files - failed to delete: %s\n", del_path); + pmd_log(PMD_LOG_ERROR, "delete_matching_iteration_files - failed to delete: %s\n", full_path); status = PMD_ERROR; goto cleanup; } @@ -1741,9 +1743,10 @@ static pmd_status delete_matching_iteration_files(const char *pattern) { } cleanup: + free(full_path); pmd_closedir(del_dir); free_iteration_pattern(&pattern_info); - return PMD_SUCCESS; + return status; } pmd_status pmd_open_series(const char *filename, pmd_series **series_out, pmd_access_mode mode) { From 6efe4e7c299ae7671086fe5ab2e00d0d376da68a Mon Sep 17 00:00:00 2001 From: "Christopher M. Pierce" Date: Tue, 6 Jan 2026 13:21:33 -0500 Subject: [PATCH 23/29] continuing to resolve memory leaks --- parcel.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/parcel.h b/parcel.h index b0a8f12..ee3da93 100644 --- a/parcel.h +++ b/parcel.h @@ -1725,6 +1725,7 @@ static pmd_status delete_matching_iteration_files(const char *pattern) { if (extract_iteration_from_name(del_entry->d_name, pattern_info.first_segment, &iter_index) == PMD_SUCCESS) { /* Reconstruct full file path from pattern (for paths like data_%T/file_%T.h5 * where we only found the directory data_1) */ + free(full_path); full_path = replace_iteration(pattern, iter_index); if (!full_path) { pmd_log(PMD_LOG_ERROR, "delete_matching_iteration_files - out of memory constructing path.\n"); @@ -1755,7 +1756,7 @@ pmd_status pmd_open_series(const char *filename, pmd_series **series_out, pmd_ac pmd_status status = PMD_SUCCESS; int is_write_mode = (mode != PMD_RDONLY); int file_exists = 0; - + char *iter_filename = NULL; iteration_pattern pattern_info; pattern_info.scan_parent = NULL; pattern_info.first_segment = NULL; @@ -1914,7 +1915,7 @@ pmd_status pmd_open_series(const char *filename, pmd_series **series_out, pmd_ac /* Don't create any files yet - will be created when iterations are added */ series->file_id = -1; } else { - char *iter_filename = replace_iteration(filename, first_iteration); + iter_filename = replace_iteration(filename, first_iteration); if (!iter_filename) { pmd_closedir(dir); status = PMD_ERROR_OUT_OF_MEMORY; @@ -2076,6 +2077,7 @@ pmd_status pmd_open_series(const char *filename, pmd_series **series_out, pmd_ac pmd_close_series(series); series = NULL; } + free(iter_filename); free_iteration_pattern(&pattern_info); *series_out = series; return status; From 7d2514ab75622a4c576be8100c74e5ce3129f65f Mon Sep 17 00:00:00 2001 From: "Christopher M. Pierce" Date: Tue, 6 Jan 2026 13:37:29 -0500 Subject: [PATCH 24/29] resolve memory leaks --- tests/test_cpp_api.cpp | 4 ++++ tests/test_generate_openpmd.c | 4 ++++ tests/test_read.c | 3 +++ tests/test_read_write.c | 3 +++ tests/test_utilities.c | 4 ++++ tests/test_write.c | 5 ++++- 6 files changed, 22 insertions(+), 1 deletion(-) diff --git a/tests/test_cpp_api.cpp b/tests/test_cpp_api.cpp index 0e0f44b..fe0d1c2 100644 --- a/tests/test_cpp_api.cpp +++ b/tests/test_cpp_api.cpp @@ -229,5 +229,9 @@ int main() { UNITY_BEGIN(); RUN_TEST(test_cpp_vector_backed_particle_group); RUN_TEST(test_cpp_selective_reading); + + /* Clean up HDF5 library internal resources */ + H5close(); + return UNITY_END(); } diff --git a/tests/test_generate_openpmd.c b/tests/test_generate_openpmd.c index 8895188..57a56a4 100644 --- a/tests/test_generate_openpmd.c +++ b/tests/test_generate_openpmd.c @@ -228,5 +228,9 @@ int main(void) { RUN_TEST(test_generate_group_based_openpmd); RUN_TEST(test_generate_file_based_openpmd); + /* Clean up HDF5 library internal resources */ + H5close(); + return UNITY_END(); } + diff --git a/tests/test_read.c b/tests/test_read.c index f157652..1d52a7a 100644 --- a/tests/test_read.c +++ b/tests/test_read.c @@ -2805,5 +2805,8 @@ int main(void) { RUN_TEST(test_windows_path_file_based_pattern); #endif + /* Clean up HDF5 library internal resources */ + H5close(); + return UNITY_END(); } diff --git a/tests/test_read_write.c b/tests/test_read_write.c index afa42a6..8e40cb4 100644 --- a/tests/test_read_write.c +++ b/tests/test_read_write.c @@ -263,5 +263,8 @@ int main(void) { RUN_TEST(test_write_and_read_particle_group_based); RUN_TEST(test_write_and_read_particle_group_file_based); + /* Clean up HDF5 library internal resources */ + H5close(); + return UNITY_END(); } diff --git a/tests/test_utilities.c b/tests/test_utilities.c index f242e25..ad35d24 100644 --- a/tests/test_utilities.c +++ b/tests/test_utilities.c @@ -232,5 +232,9 @@ int main(void) { RUN_TEST(test_parse_iteration_pattern); RUN_TEST(test_extract_iteration_from_name); RUN_TEST(test_replace_iteration); + + /* Clean up HDF5 library internal resources */ + H5close(); + return UNITY_END(); } diff --git a/tests/test_write.c b/tests/test_write.c index 27f4045..57bb272 100644 --- a/tests/test_write.c +++ b/tests/test_write.c @@ -2572,7 +2572,7 @@ void test_windows_path_rdwr(void) { int main(void) { /* Suppress error messages during tests */ H5Eset_auto2(H5E_DEFAULT, NULL, NULL); - //pmd_set_log_level(PMD_LOG_NONE); + pmd_set_log_level(PMD_LOG_NONE); UNITY_BEGIN(); @@ -2620,5 +2620,8 @@ int main(void) { RUN_TEST(test_windows_path_rdwr); #endif + /* Clean up HDF5 library internal resources */ + H5close(); + return UNITY_END(); } From dfe141127a4d122b53374998fc674965f7def425 Mon Sep 17 00:00:00 2001 From: "Christopher M. Pierce" Date: Tue, 6 Jan 2026 13:42:21 -0500 Subject: [PATCH 25/29] surpress error messages --- tests/test_read.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_read.c b/tests/test_read.c index 1d52a7a..b523d49 100644 --- a/tests/test_read.c +++ b/tests/test_read.c @@ -2710,6 +2710,7 @@ void test_user_supplied_arrays(void) { int main(void) { // Turn off logging for tests pmd_set_log_level(PMD_LOG_NONE); + H5Eset_auto2(H5E_DEFAULT, NULL, NULL); UNITY_BEGIN(); From 720c1b9b594e438874b3dffcc98b0ceaebacf555 Mon Sep 17 00:00:00 2001 From: "Christopher M. Pierce" Date: Tue, 6 Jan 2026 13:42:35 -0500 Subject: [PATCH 26/29] surpress error messages --- tests/test_read_write.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_read_write.c b/tests/test_read_write.c index 8e40cb4..f45b79b 100644 --- a/tests/test_read_write.c +++ b/tests/test_read_write.c @@ -256,6 +256,7 @@ void test_write_and_read_particle_group_file_based(void) { int main(void) { /* Suppress HDF5 error messages during tests */ + pmd_set_log_level(PMD_LOG_NONE); H5Eset_auto2(H5E_DEFAULT, NULL, NULL); UNITY_BEGIN(); From 12edd28f3c18fb996624cc541063295113b561a9 Mon Sep 17 00:00:00 2001 From: "Christopher M. Pierce" Date: Tue, 6 Jan 2026 13:50:55 -0500 Subject: [PATCH 27/29] add test for windows path separator in iterationFormat --- tests/test_write.c | 115 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 115 insertions(+) diff --git a/tests/test_write.c b/tests/test_write.c index 57bb272..0dce732 100644 --- a/tests/test_write.c +++ b/tests/test_write.c @@ -2567,6 +2567,120 @@ void test_windows_path_rdwr(void) { result = pmd_close_series(series); TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, result); } + +/** + * Test: Verify iterationFormat uses Unix-style path separators + * Even when Windows paths are used, iterationFormat should use forward slashes internally + */ +void test_windows_iteration_format_normalized(void) { + typedef struct { + const char *pattern; + const char *expected_iteration_format; + const char *description; + } test_case; + + test_case cases[] = { + { + "tests\\temp_writer\\norm_%T.h5", + "norm_%T.h5", + "Simple filename with backslash dir" + }, + { + "tests\\temp_writer\\norm_%T\\data.h5", + "norm_%T/data.h5", + "Directory with %T using backslashes" + }, + { + "tests\\temp_writer\\norm_%T\\file_%T.h5", + "norm_%T/file_%T.h5", + "Multiple %T across directory boundary with backslashes" + }, + { + "tests\\temp_writer\\norm_%T\\step_%T\\data.h5", + "norm_%T/step_%T/data.h5", + "Nested directories with backslashes" + } + }; + + int num_cases = sizeof(cases) / sizeof(cases[0]); + + for (int i = 0; i < num_cases; i++) { + pmd_series *series; + pmd_iteration *iter; + pmd_status result; + + /* Create file-based series with Windows path */ + result = pmd_open_series(cases[i].pattern, &series, PMD_TRUNC); + TEST_ASSERT_EQUAL_INT_MESSAGE(PMD_SUCCESS, result, cases[i].description); + TEST_ASSERT_EQUAL_INT_MESSAGE(PMD_FILE_BASED, series->iteration_encoding, cases[i].description); + + /* Create an iteration to ensure file is written */ + result = pmd_open_iteration(series, 5, &iter); + TEST_ASSERT_EQUAL_INT_MESSAGE(PMD_SUCCESS, result, cases[i].description); + result = pmd_close_iteration(iter); + TEST_ASSERT_EQUAL_INT_MESSAGE(PMD_SUCCESS, result, cases[i].description); + + result = pmd_close_series(series); + TEST_ASSERT_EQUAL_INT_MESSAGE(PMD_SUCCESS, result, cases[i].description); + + /* Open the created file directly with HDF5 and read iterationFormat attribute */ + char *actual_file = replace_iteration(cases[i].pattern, 5); + TEST_ASSERT_NOT_NULL_MESSAGE(actual_file, cases[i].description); + + /* Normalize path for opening (Windows allows both) */ + for (char *p = actual_file; *p; p++) { + if (*p == '\\') *p = '/'; + } + + hid_t file_id = H5Fopen(actual_file, H5F_ACC_RDONLY, H5P_DEFAULT); + char msg[256]; + snprintf(msg, sizeof(msg), "%s - failed to open: %s", cases[i].description, actual_file); + TEST_ASSERT_MESSAGE(file_id >= 0, msg); + + /* Read iterationFormat attribute */ + hid_t attr_id = H5Aopen(file_id, "iterationFormat", H5P_DEFAULT); + snprintf(msg, sizeof(msg), "%s - iterationFormat attribute not found", cases[i].description); + TEST_ASSERT_MESSAGE(attr_id >= 0, msg); + + hid_t type_id = H5Aget_type(attr_id); + htri_t is_variable = H5Tis_variable_str(type_id); + char *iteration_format = NULL; + + if (is_variable > 0) { + /* Variable-length string */ + char *vlen_str = NULL; + H5Aread(attr_id, type_id, &vlen_str); + iteration_format = strdup(vlen_str); + H5free_memory(vlen_str); + } else { + /* Fixed-length string */ + size_t size = H5Tget_size(type_id); + iteration_format = (char *)malloc(size + 1); + TEST_ASSERT_NOT_NULL(iteration_format); + H5Aread(attr_id, type_id, iteration_format); + iteration_format[size] = '\0'; + } + + TEST_ASSERT_NOT_NULL_MESSAGE(iteration_format, cases[i].description); + + /* Verify it uses forward slashes */ + snprintf(msg, sizeof(msg), "%s - Expected '%s', got '%s'", + cases[i].description, cases[i].expected_iteration_format, iteration_format); + TEST_ASSERT_EQUAL_STRING_MESSAGE(cases[i].expected_iteration_format, iteration_format, msg); + + /* Verify no backslashes in iterationFormat */ + snprintf(msg, sizeof(msg), "%s - iterationFormat contains backslash: %s", + cases[i].description, iteration_format); + TEST_ASSERT_NULL_MESSAGE(strchr(iteration_format, '\\'), msg); + + /* Clean up */ + free(iteration_format); + H5Tclose(type_id); + H5Aclose(attr_id); + H5Fclose(file_id); + free(actual_file); + } +} #endif /* _WIN32 */ int main(void) { @@ -2618,6 +2732,7 @@ int main(void) { RUN_TEST(test_windows_path_group_based_write); RUN_TEST(test_windows_path_file_based_pattern_write); RUN_TEST(test_windows_path_rdwr); + RUN_TEST(test_windows_iteration_format_normalized); #endif /* Clean up HDF5 library internal resources */ From b580e7575d12739657e15a09bf6241429eaeb9ef Mon Sep 17 00:00:00 2001 From: "Christopher M. Pierce" Date: Tue, 6 Jan 2026 13:57:36 -0500 Subject: [PATCH 28/29] normalize path before placing into file --- parcel.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/parcel.h b/parcel.h index ee3da93..7caf4de 100644 --- a/parcel.h +++ b/parcel.h @@ -1699,6 +1699,20 @@ static pmd_status read_series_metadata_from_file(hid_t file_id, pmd_series *seri return PMD_SUCCESS; } +/** + * Normalize path separators to forward slashes (in-place) + * Converts Windows-style backslashes to Unix-style forward slashes + * for cross-platform compatibility in stored metadata + */ +static void normalize_path_separators(char *path) { + if (!path) return; + for (char *p = path; *p; p++) { + if (*p == '\\') { + *p = '/'; + } + } +} + /** * Delete all iteration files matching the pattern * Used during truncate mode to remove existing iterations @@ -1872,6 +1886,7 @@ pmd_status pmd_open_series(const char *filename, pmd_series **series_out, pmd_ac } } series->iteration_format = strdup(filename_pattern); + normalize_path_separators(series->iteration_format); series->base_path = strdup("/data/%T/"); series->_particles_path = strdup("particles/"); @@ -1909,6 +1924,7 @@ pmd_status pmd_open_series(const char *filename, pmd_series **series_out, pmd_ac } } series->iteration_format = strdup(filename_pattern); + normalize_path_separators(series->iteration_format); series->base_path = strdup("/data/%T/"); series->_particles_path = strdup("particles/"); From 212cc106e24330d5248b14232b2bd384db14cc71 Mon Sep 17 00:00:00 2001 From: "Christopher M. Pierce" Date: Tue, 6 Jan 2026 17:12:45 -0500 Subject: [PATCH 29/29] clean up log statements --- parcel.h | 75 ++++++++++++++++++++++++++-------------------- tests/test_read.c | 4 +-- tests/test_write.c | 2 +- 3 files changed, 45 insertions(+), 36 deletions(-) diff --git a/parcel.h b/parcel.h index 7caf4de..c66870d 100644 --- a/parcel.h +++ b/parcel.h @@ -657,6 +657,8 @@ void pmd_set_log_level(pmd_log_level level) { * @param ... Variable arguments for format string */ static void pmd_log(pmd_log_level level, const char *format, ...) { + int status; + if (level > pmd_log_threshold) { return; /* Message level below threshold, suppress */ } @@ -673,16 +675,25 @@ static void pmd_log(pmd_log_level level, const char *format, ...) { va_list args; va_start(args, format); - // Construct - int fstatus = fprintf(stderr, "%s: ", level_str); - if ((fstatus < 0) && (pmd_log_threshold > PMD_LOG_NONE)) { - printf("pmd_log - failed to construct log level message."); + /* Construct start of log line */ + status = fprintf(stderr, "%s: ", level_str); + if ((status < 0) && (pmd_log_threshold > PMD_LOG_NONE)) { + printf("pmd_log - failed to construct log level message.\n"); + return; + } + + /* Perform user formatting */ + status = vfprintf(stderr, format, args); + if ((status < 0) && (pmd_log_threshold > PMD_LOG_NONE)) { + printf("pmd_log - failed to apply user formatting to log message.\n"); + return; } - // Perform user formatting - int vstatus = vfprintf(stderr, format, args); - if ((vstatus < 0) && (pmd_log_threshold > PMD_LOG_NONE)) { - printf("pmd_log - failed to apply user formatting to log message."); + /* Append newline to each message*/ + status = fprintf(stderr, "\n"); + if ((status < 0) && (pmd_log_threshold > PMD_LOG_NONE)) { + printf("pmd_log - failed to add newline to log message.\n"); + return; } va_end(args); @@ -917,7 +928,7 @@ static pmd_status read_string_attribute(hid_t loc_id, const char *attr_name, cha /* Check if attribute exists */ if (attribute_exists(loc_id, attr_name) <= 0) { - pmd_log(PMD_LOG_ERROR, "Missing '%s' attribute\n", attr_name); + pmd_log(PMD_LOG_ERROR, "Missing '%s' attribute", attr_name); return PMD_ERROR_FILE_FORMAT; } @@ -1542,7 +1553,7 @@ static pmd_status read_series_metadata_from_file(hid_t file_id, pmd_series *seri int minor; int revision; if (sscanf(openpmd_version, "%d.%d.%d", &major, &minor, &revision) != 3) { - pmd_log(PMD_LOG_ERROR, "Invalid OpenPMD version format '%s' in '%s' (expected X.Y.Z)\n", + pmd_log(PMD_LOG_ERROR, "Invalid OpenPMD version format '%s' in '%s' (expected X.Y.Z)", openpmd_version, filename); free(openpmd_version); return PMD_ERROR_FILE_FORMAT; @@ -1551,7 +1562,7 @@ static pmd_status read_series_metadata_from_file(hid_t file_id, pmd_series *seri /* Warn if major version is greater than 2 (our implementation target) */ if (major > 2) { pmd_log(PMD_LOG_WARNING, "File '%s' uses OpenPMD version %d.%d.%d, but this library implements version 2.x.x " - "Some features may not be supported or may behave unexpectedly.\n", + "Some features may not be supported or may behave unexpectedly.", filename, major, minor, revision); } @@ -1570,7 +1581,7 @@ static pmd_status read_series_metadata_from_file(hid_t file_id, pmd_series *seri return status; } } else { - pmd_log(PMD_LOG_WARNING, "Missing 'iterationFormat' attribute in '%s', using basePath as default\n", filename); + pmd_log(PMD_LOG_WARNING, "Missing 'iterationFormat' attribute in '%s', using basePath as default", filename); series->iteration_format = strdup(series->base_path); } @@ -1581,7 +1592,7 @@ static pmd_status read_series_metadata_from_file(hid_t file_id, pmd_series *seri return status; } } else { - pmd_log(PMD_LOG_WARNING, "Missing 'iterationEncoding' attribute in '%s', defaulting to 'groupBased'\n", filename); + pmd_log(PMD_LOG_WARNING, "Missing 'iterationEncoding' attribute in '%s', defaulting to 'groupBased'", filename); iter_encoding_str = strdup("groupBased"); } @@ -1742,7 +1753,7 @@ static pmd_status delete_matching_iteration_files(const char *pattern) { free(full_path); full_path = replace_iteration(pattern, iter_index); if (!full_path) { - pmd_log(PMD_LOG_ERROR, "delete_matching_iteration_files - out of memory constructing path.\n"); + pmd_log(PMD_LOG_ERROR, "delete_matching_iteration_files - out of memory constructing path."); status = PMD_ERROR_OUT_OF_MEMORY; goto cleanup; } @@ -1750,7 +1761,7 @@ static pmd_status delete_matching_iteration_files(const char *pattern) { /* Delete the file or directory */ int rstatus = remove(full_path); if (rstatus != 0) { - pmd_log(PMD_LOG_ERROR, "delete_matching_iteration_files - failed to delete: %s\n", full_path); + pmd_log(PMD_LOG_ERROR, "delete_matching_iteration_files - failed to delete: %s", full_path); status = PMD_ERROR; goto cleanup; } @@ -3964,7 +3975,7 @@ pmd_status pmd_read_particle_group(pmd_iteration *iter, const char *species, H5O_info2_t obj_info; if (H5Oget_info_by_name(species_group_id, "position", &obj_info, H5O_INFO_BASIC, H5P_DEFAULT) >= 0) { if (obj_info.type != H5O_TYPE_GROUP) { - pmd_log(PMD_LOG_ERROR, "pmd_read_particle_group: 'position' is not a group\n"); + pmd_log(PMD_LOG_ERROR, "pmd_read_particle_group: 'position' is not a group"); status = PMD_ERROR_FILE_FORMAT; goto cleanup; } @@ -3992,7 +4003,7 @@ pmd_status pmd_read_particle_group(pmd_iteration *iter, const char *species, status = read_double_record(species_group_id, "positionOffset/x", pg->x_offset, num_particles, 1.0); if (status != PMD_SUCCESS) goto cleanup; } else { - pmd_log(PMD_LOG_WARNING, "pmd_read_particle_group: positionOffset/x not found, using zeros\n"); + pmd_log(PMD_LOG_WARNING, "pmd_read_particle_group: positionOffset/x not found, using zeros"); for (int64_t i = 0; i < num_particles; i++) pg->x_offset[i] = 0.0; } } @@ -4002,7 +4013,7 @@ pmd_status pmd_read_particle_group(pmd_iteration *iter, const char *species, status = read_double_record(species_group_id, "positionOffset/y", pg->y_offset, num_particles, 1.0); if (status != PMD_SUCCESS) goto cleanup; } else { - pmd_log(PMD_LOG_WARNING, "pmd_read_particle_group: positionOffset/y not found, using zeros\n"); + pmd_log(PMD_LOG_WARNING, "pmd_read_particle_group: positionOffset/y not found, using zeros"); for (int64_t i = 0; i < num_particles; i++) pg->y_offset[i] = 0.0; } } @@ -4012,7 +4023,7 @@ pmd_status pmd_read_particle_group(pmd_iteration *iter, const char *species, status = read_double_record(species_group_id, "positionOffset/z", pg->z_offset, num_particles, 1.0); if (status != PMD_SUCCESS) goto cleanup; } else { - pmd_log(PMD_LOG_WARNING, "pmd_read_particle_group: positionOffset/z not found, using zeros\n"); + pmd_log(PMD_LOG_WARNING, "pmd_read_particle_group: positionOffset/z not found, using zeros"); for (int64_t i = 0; i < num_particles; i++) pg->z_offset[i] = 0.0; } } @@ -4032,7 +4043,7 @@ pmd_status pmd_read_particle_group(pmd_iteration *iter, const char *species, H5O_info2_t obj_info; if (H5Oget_info_by_name(species_group_id, "momentum", &obj_info, H5O_INFO_BASIC, H5P_DEFAULT) >= 0) { if (obj_info.type != H5O_TYPE_GROUP) { - pmd_log(PMD_LOG_ERROR, "pmd_read_particle_group: 'momentum' is not a group\n"); + pmd_log(PMD_LOG_ERROR, "pmd_read_particle_group: 'momentum' is not a group"); status = PMD_ERROR_FILE_FORMAT; goto cleanup; } @@ -4373,30 +4384,30 @@ pmd_status pmd_write_particle_group(pmd_iteration *iter, const particle_group *p /* Validate inputs */ if (!iter || !pg) return PMD_ERROR_NULL_POINTER; if (!pg->species_type) { - pmd_log(PMD_LOG_ERROR, "species_type is required\n"); + pmd_log(PMD_LOG_ERROR, "species_type is required"); return PMD_ERROR_NULL_POINTER; } /* Check write mode */ if (iter->series->access_mode == PMD_RDONLY) { - pmd_log(PMD_LOG_ERROR, "Cannot write in read-only mode\n"); + pmd_log(PMD_LOG_ERROR, "Cannot write in read-only mode"); return PMD_ERROR; } /* Validate required position fields */ if (!pg->x || !pg->y || !pg->z) { - pmd_log(PMD_LOG_ERROR, "Position arrays (x, y, z) are required\n"); + pmd_log(PMD_LOG_ERROR, "Position arrays (x, y, z) are required"); return PMD_ERROR_NULL_POINTER; } if (pg->num_particles <= 0) { - pmd_log(PMD_LOG_ERROR, "num_particles must be positive\n"); + pmd_log(PMD_LOG_ERROR, "num_particles must be positive"); return PMD_ERROR; } /* Get particles path */ status = pmd_get_particles_path(iter->series, &particles_path); if (status != PMD_SUCCESS) { - pmd_log(PMD_LOG_ERROR, "Series has no particlesPath\n"); + pmd_log(PMD_LOG_ERROR, "Series has no particlesPath"); return PMD_ERROR; } @@ -4540,7 +4551,7 @@ static pmd_status validate_attribute_type(hid_t attr_id, H5T_class_t expected_cl H5Tclose(attr_type); if (type_class != expected_class) { - pmd_log(PMD_LOG_ERROR, "validate_attribute_type: Attribute has type class %d, expected %d\n", + pmd_log(PMD_LOG_ERROR, "validate_attribute_type: Attribute has type class %d, expected %d", type_class, expected_class); return PMD_ERROR_FILE_FORMAT; } @@ -4591,7 +4602,7 @@ static pmd_status read_unit_si(hid_t loc_id, double *unit_si_out) { } } else { /* Wrong type */ - pmd_log(PMD_LOG_ERROR, "read_unit_si: 'unitSI' attribute has type class %d, expected float or integer\n", type_class); + pmd_log(PMD_LOG_ERROR, "read_unit_si: 'unitSI' attribute has type class %d, expected float or integer", type_class); H5Aclose(attr_id); return PMD_ERROR_FILE_FORMAT; } @@ -4637,7 +4648,7 @@ static pmd_status read_record_generic(hid_t group_id, const char *name, if (ndims != 1) { H5Sclose(dataspace_id); H5Dclose(dataset_id); - pmd_log(PMD_LOG_ERROR, "Dataset '%s' has rank %d, expected 1\n", name, ndims); + pmd_log(PMD_LOG_ERROR, "Dataset '%s' has rank %d, expected 1", name, ndims); return PMD_ERROR_FILE_FORMAT; } @@ -4647,7 +4658,7 @@ static pmd_status read_record_generic(hid_t group_id, const char *name, if (dims[0] != (hsize_t)num_particles) { H5Sclose(dataspace_id); H5Dclose(dataset_id); - pmd_log(PMD_LOG_ERROR, "Dataset '%s' has size %llu, expected %lld\n", + pmd_log(PMD_LOG_ERROR, "Dataset '%s' has size %llu, expected %lld", name, dims[0], num_particles); return PMD_ERROR_FILE_FORMAT; } @@ -4680,7 +4691,7 @@ static pmd_status read_record_generic(hid_t group_id, const char *name, /* Check if 'value' attribute exists before trying to open it */ if (attribute_exists(group_id_local, "value") <= 0) { /* Group exists but no 'value' attribute - format error */ - pmd_log(PMD_LOG_ERROR, "read_record_generic: Constant record '%s' missing 'value' attribute\n", name); + pmd_log(PMD_LOG_ERROR, "read_record_generic: Constant record '%s' missing 'value' attribute", name); H5Gclose(group_id_local); return PMD_ERROR_FILE_FORMAT; } @@ -4701,7 +4712,7 @@ static pmd_status read_record_generic(hid_t group_id, const char *name, if (expected_class != H5T_NO_CLASS) { pmd_status type_status = validate_attribute_type(attr_id, expected_class); if (type_status != PMD_SUCCESS) { - pmd_log(PMD_LOG_ERROR, "read_record_generic: Constant record '%s' has wrong type for 'value' attribute\n", name); + pmd_log(PMD_LOG_ERROR, "read_record_generic: Constant record '%s' has wrong type for 'value' attribute", name); H5Aclose(attr_id); H5Gclose(group_id_local); return type_status; @@ -4721,7 +4732,7 @@ static pmd_status read_record_generic(hid_t group_id, const char *name, if (space_type != H5S_SCALAR) { /* value must be a scalar, not an array */ - pmd_log(PMD_LOG_ERROR, "read_record_generic: Constant record '%s' has array 'value', expected scalar\n", name); + pmd_log(PMD_LOG_ERROR, "read_record_generic: Constant record '%s' has array 'value', expected scalar", name); H5Aclose(attr_id); H5Gclose(group_id_local); return PMD_ERROR_FILE_FORMAT; diff --git a/tests/test_read.c b/tests/test_read.c index b523d49..3d174f4 100644 --- a/tests/test_read.c +++ b/tests/test_read.c @@ -457,9 +457,7 @@ void test_group_based_series_multiple_iterations(void) { /* Open each iteration and verify we can access it */ for (int i = 0; i < 3; i++) { pmd_iteration *iter; - printf("iteration %d: %d\n", i, iterations[i]); result = pmd_open_iteration(series, iterations[i], &iter); - printf("iteration %d: %d\n", i, iterations[i]); TEST_ASSERT_EQUAL_INT(PMD_SUCCESS, result); TEST_ASSERT_NOT_NULL(iter); TEST_ASSERT_EQUAL_INT64(iterations[i], iter->iteration_index); @@ -2709,7 +2707,7 @@ void test_user_supplied_arrays(void) { /* Main test runner */ int main(void) { // Turn off logging for tests - pmd_set_log_level(PMD_LOG_NONE); + //pmd_set_log_level(PMD_LOG_NONE); H5Eset_auto2(H5E_DEFAULT, NULL, NULL); UNITY_BEGIN(); diff --git a/tests/test_write.c b/tests/test_write.c index 0dce732..b3fd0ee 100644 --- a/tests/test_write.c +++ b/tests/test_write.c @@ -2686,7 +2686,7 @@ void test_windows_iteration_format_normalized(void) { int main(void) { /* Suppress error messages during tests */ H5Eset_auto2(H5E_DEFAULT, NULL, NULL); - pmd_set_log_level(PMD_LOG_NONE); + //pmd_set_log_level(PMD_LOG_NONE); UNITY_BEGIN();