Skip to content

BUG: Invoking SetData method with wrong parameter types - #6717

Merged
dzenanz merged 1 commit into
InsightSoftwareConsortium:mainfrom
dzenanz:arrayProblemsInTest
Jul 29, 2026
Merged

BUG: Invoking SetData method with wrong parameter types#6717
dzenanz merged 1 commit into
InsightSoftwareConsortium:mainfrom
dzenanz:arrayProblemsInTest

Conversation

@dzenanz

@dzenanz dzenanz commented Jul 28, 2026

Copy link
Copy Markdown
Member

The signature is void SetData(TValue * datain, SizeValueType sz, bool LetArrayManageMemory)

This, by itself, is not a big problem. However, it might be a symptom of a larger problem.

PR Checklist

  • No API changes were made (or the changes have been approved)
  • No major design changes were made (or the changes have been approved)
  • Added test (or behavior not changed)

The signature is void SetData(TValue * datain, SizeValueType sz, bool LetArrayManageMemory)
@dzenanz
dzenanz requested a review from hjmjohnson July 28, 2026 20:05
@github-actions github-actions Bot added type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct area:Core Issues affecting the Core module labels Jul 28, 2026
@greptile-apps

greptile-apps Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Corrects two memory-management test calls to pass the array size explicitly to SetData.

  • Prevents false from being implicitly interpreted as a zero size.
  • Preserves non-owning use of the stack-allocated buffer.

Confidence Score: 5/5

The PR appears safe to merge.

The changed test calls supply the required buffer length while retaining non-owning memory semantics, and no actionable failures remain.

T-Rex T-Rex Logs

What T-Rex did

  • The prerequisites for a Debug build were verified as missing, with cmake, ctest, and ninja not in PATH and no tooling installed.
  • An attempt was made to run the requested Debug configuration command, which exited with code 127 because cmake was not found.

View all artifacts

T-Rex Ran code and verified through T-Rex

Important Files Changed

Filename Overview
Modules/Core/Common/test/itkArrayGTest.cxx Both corrected calls now match the intended SetData(buffer, size, manageMemory) contract without changing production code.

Reviews (1): Last reviewed commit: "BUG: Invoking SetData method with wrong ..." | Re-trigger Greptile

@hjmjohnson

Copy link
Copy Markdown
Member

Confirmed correct, and worth landing as-is. The SetData(buffer, false) form promotes false to SizeValueType 0 with no diagnostic under -Wall -Wextra -Wconversion -Wsign-conversion, so both call sites were operating on empty arrays; the assertions could not detect it because they compare a size against the size it was just copied from.

Follow-up analysis and recommended paths recorded in #6718 — the API hazard that produced these two lines is still present, and itk::VariableLengthVector still carries the overload pair itk::Array dropped in 9e49264 (2016). This PR is a prerequisite for the Array guard proposed there.

@dzenanz
dzenanz merged commit 0637d3c into InsightSoftwareConsortium:main Jul 29, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Core Issues affecting the Core module type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants