Skip to content

BUG: Backport null checks after ReadHeader in IPLCommonImageIO #6064

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Open
hjmjohnson wants to merge 1 commit into
base: release-5.4
Choose a base branch
from
+8 −3
Open

BUG: Backport null checks after ReadHeader in IPLCommonImageIO #6064

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions Modules/IO/IPL/src/itkIPLCommonImageIO.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,10 @@ IPLCommonImageIO::ReadImageInformation()
FileNameToRead = _imagePath;

this->m_ImageHeader = this->ReadHeader(FileNameToRead.c_str());
//
// if anything fails in the header read, just let
// exceptions propagate up.
if (m_ImageHeader == nullptr)
{
itkExceptionMacro("ReadHeader failed for " << FileNameToRead);
}

bool isCT = false;
std::string modality = m_ImageHeader->modality;
Expand Down Expand Up @@ -224,6 +225,10 @@ IPLCommonImageIO::ReadImageInformation()
// throw an exception, and we'd just want to skip it.
continue;
}
if (curImageHeader == nullptr)
{
itkExceptionMacro("ReadHeader failed for " << fullPath);
}
Comment on lines +228 to +231
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Apr 15, 2026

Choose a reason for hiding this comment

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

P2 Loop null-check aborts scan on one unexpected file

The null check correctly prevents a crash, but the itkExceptionMacro throw sits outside the enclosing try-catch block (lines 217-227). If any file in the directory has a ReadHeader implementation that returns nullptr without throwing — the base class IPLCommonImageIO::ReadHeader does exactly this — the exception will propagate all the way up and abort the entire ReadImageInformation() call, rather than skipping that file as the surrounding try-catch intends.

All three known child-class implementations (GE4, GE5, GEAdw) always throw on failure, so this is unlikely to trigger in practice. But using continue here would be consistent with the loop's error-handling philosophy and safe against future subclasses:

Suggested change
if (curImageHeader == nullptr)
{
itkExceptionMacro("ReadHeader failed for " << fullPath);
}
if (curImageHeader == nullptr)
{
continue;
}

Copy link
Copy Markdown
Member Author

@hjmjohnson hjmjohnson Apr 15, 2026

Choose a reason for hiding this comment

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

This is a backport from main -> release-5.4; if another developer thinks it needs to be fixed, then we need to fix it upstream as well.

if ((((isCT) ? curImageHeader->examNumber : curImageHeader->echoNumber) == m_FilenameList->GetKey2()) &&
(curImageHeader->seriesNumber == m_FilenameList->GetKey1()))
{
Expand Down
Loading