Skip to content

Comprensive fixes of warnings and static analysis bugs#142

Closed
gdevenyi wants to merge 25 commits intoBIC-MNI:developfrom
gdevenyi:static-fixes-claude
Closed

Comprensive fixes of warnings and static analysis bugs#142
gdevenyi wants to merge 25 commits intoBIC-MNI:developfrom
gdevenyi:static-fixes-claude

Conversation

@gdevenyi
Copy link
Copy Markdown
Contributor

gcc-15 and clang-19 full warnings, plus valgrind and scan-build-19, evaluated with Opus 4.6 under maximum effort reasoning. Rebuild and tests run after each bugfix.

Each commit fixes a single bug for ease of cherry-picking/dropping.

gdevenyi and others added 25 commits March 5, 2026 12:56
Remove va_end() call from v_mi2log_message() - callers already call
va_end, so having it inside the function caused undefined behavior
(double va_end). Also, the va_end was only called conditionally
inside an if-block, so some paths never called it at all.

In milog_init(), close a previously-opened log file before
re-initializing to prevent FILE* leaks, and fall back to stderr
if fopen() fails.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Change mierror_table msgfmt field from char* to const char*
- Change minc_routine_name from char* to const char*
- Update MI_save_routine_name, MI_log_pkg_error2/3, MI_log_sys_error1
  and their MI2_ counterparts to take const char* parameters

Fixes ~110 -Wdiscarded-qualifiers warnings from callers passing
string literals to these functions.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Return NULL if malloc fails instead of dereferencing a NULL pointer.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…bsrc

- MI_get_default_range: const char *what parameter
- hdf_is_dimension_name: const char *dimnms[] array
- minc_simple.c: const char *minc_dimnames[] and *dimensions[]
- minc_convenience.c: const char *att_sign, *attname, *prefix_string
- netcdf_convenience.c: const char *extension in compression_code_list,
  const char *command in execute_decompress_command, const char *tmpdir_ptr

Fixes -Wdiscarded-qualifiers warnings where string literal macros
(MIxspace, MI_SIGNED, etc.) were assigned to non-const char* variables.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
alloc1d() and alloc2d() can return NULL on allocation failure.
The gaussian elimination and matrix inversion functions were
dereferencing these pointers without checking, which is undefined
behavior on allocation failure.

Also make free2d() NULL-safe for consistency with free().

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
hdf_dim_add() and hdf_var_add() can return NULL if the variable
limit is exceeded or malloc fails. The code in hdf_open was
dereferencing these return values without checking.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Rename inner 'size' to 'fill_size' to avoid shadowing the outer
'size' variable in the same function.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- input_mnc2.c: Remove slices_count which was computed but never read
- output_mnc2.c: Remove slab_size which was computed but never read
- minc2-grpattr-test.c: Remove count which was incremented but never read

Fixes -Wunused-but-set-variable warnings from clang-19.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Restore the count variable and its increment for tracking iteration
in the attribute listing loop.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Split the dual-purpose 'name' variable: keep 'name' for the malloc'd
buffer used for dimension-width dataset names, add 'attr_str' as
const char* for the string literal assignments (spacing, class, etc.).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The function only reads dimension names (via strcmp), so the names
parameter can safely be const. This allows callers to use const char*
arrays initialized with MI dimension name macros without warnings.

Update all test callers to use const char* arrays.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ISO C forbids an empty translation unit. Add a dummy typedef since
all code was moved to minc_error.c.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The target functions (ncvardef/MI2vardef, micreate_std_variable) already
accept const char* parameters, so casting string literals to char* was
both unnecessary and discarded const-qualification.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…2-large-attribute)

Add const to string pointer declarations that are initialized with
string literals to eliminate discarded-qualifiers warnings.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fix analyzer-detected possible null dereferences (CWE-690):
- grpattr.c: Check malloc return for milistframe and std_name
- dimension.c: Check malloc return for widths array
- voxel_loop.c: Check MALLOC returns in create_loop_options,
  create_loop_info, initialize_loopfile_info, and history append
- input_mnc2.c: Check calloc return for slice_start
- minc_error.c: Guard against NULL from miget_cfg_str

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Check malloc return values before dereferencing allocated buffers in
test programs to fix analyzer-detected possible null dereferences.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ce warnings

- voxel_loop.c: Check all inner MALLOC calls in initialize_loopfile_info
  and clean up properly on failure
- datatype.c: Check malloc for spacetype name string
- dimension.c: Check malloc for dim_indices array (both occurrences)
- volume.c: Check malloc for dimension handle in _miset_volume_class
- netcdf_convenience.c: Check MALLOC for compressed filename buffer

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- hdf_convenience.c: Initialize *ndims = 0 on error path in
  hdf_get_diminfo to prevent caller using uninitialized value
- volume_cache.c: Initialize block_index and block_start to prevent
  use-of-uninitialized-value warnings from switch fallthrough and
  cross-function analysis

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- minc2-create-test-images.c: Use goto-based cleanup in create_2D/3D/4D_image
  to free allocated buffers on early error returns
- icv_vec.c: Free both allocations when either malloc fails
- dimension.c: Free strdup'd name/comments/offsets/units when later
  allocations fail in micopy_dimension and micreate_dimension

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- m2util.c: Free previously allocated rows when a later row allocation
  fails in alloc2d, preventing memory leak
- minc2-read-rgb.c: Add NULL checks after malloc for dim/sizes/origin/step
  arrays

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Both variables are set but never read, triggering -Wunused-but-set-variable.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- minc_error.c: Change strerror result variable to const char *
- hdf_convenience.c: Change hdf_open_dsets cpath param to const char *
- minc.h/netcdf_convenience.c: Change miattget_with_sign insign param
  to const char *
- minc_simple.c: Change signstr variables and minc_simple_to_nc_type
  param to const char *

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add NULL check after malloc for temp_buffer
- Free temp_buffer on early return paths (volume alloc failure,
  short read) to prevent memory leaks

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ru_maxrss is reported in bytes on macOS but kilobytes on Linux.
The leak-detection threshold was implicitly calibrated for KB,
causing a spurious "MEMORY LEAK DETECTED" on macOS (especially
ARM runners) where values are 1024x larger.

Normalize ru_maxrss to KB on macOS via #ifdef __APPLE__.

References:
- darwin-xnu getrusage.2: "in bytes"
  https://github.com/apple/darwin-xnu/blob/main/bsd/man/man2/getrusage.2
- Linux getrusage(2): "in KiB"
  https://man7.org/linux/man-pages/man2/getrusage.2.html

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vfonov
Copy link
Copy Markdown
Member

vfonov commented Mar 24, 2026

Why not develop branch?

@gdevenyi
Copy link
Copy Markdown
Contributor Author

Why not develop branch?

I didn't know that was the workflow. Happy to fix it.

@gdevenyi gdevenyi changed the base branch from master to develop March 24, 2026 15:00
@gdevenyi gdevenyi marked this pull request as draft March 24, 2026 15:03
@gdevenyi gdevenyi closed this Mar 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants