Skip to content

Use integer division in TiffBinaryArray::doCount#9288

Merged
neheb merged 1 commit intoExiv2:mainfrom
MarkLee131:fix/tiff-docount-integer-division
Apr 6, 2026
Merged

Use integer division in TiffBinaryArray::doCount#9288
neheb merged 1 commit intoExiv2:mainfrom
MarkLee131:fix/tiff-docount-integer-division

Conversation

@MarkLee131
Copy link
Copy Markdown
Contributor

Fix #9287:

doCount() uses std::lround(double(size) / typeSize) which rounds to the nearest integer. When size is not a multiple of typeSize, this can round up and report more elements than the data holds. On re-read, the inflated count causes the parser to expect more data than exists.

Use integer division instead, which truncates and is consistent with how readTiffEntry computes size from count and typeSize.

doCount() uses std::lround(double(size) / typeSize) which rounds
to the nearest integer. When size is not a multiple of typeSize,
this can round up and report more elements than the data holds.
On re-read, the inflated count causes the parser to expect more
data than exists.

Use integer division instead, which truncates and is consistent
with how readTiffEntry computes size from count and typeSize.
Copilot AI review requested due to automatic review settings March 29, 2026 15:15
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes incorrect element-count computation for TiffBinaryArray when the underlying byte size is not an exact multiple of the element type size, preventing write/re-read failures (issue #9287).

Changes:

  • Replace floating-point rounding (std::lround) with integer division for TiffBinaryArray::doCount().

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

}

return std::lround(static_cast<double>(size()) / typeSize);
return size() / typeSize;
Copy link

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

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

With std::lround removed, now appears unused in this translation unit. Consider dropping the include to avoid unnecessary header dependencies/build time.

Copilot uses AI. Check for mistakes.
}

return std::lround(static_cast<double>(size()) / typeSize);
return size() / typeSize;
Copy link

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

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

This change fixes a subtle write/re-read mismatch for malformed TIFF data. Please add a regression test that writes a TIFF with a TiffBinaryArray whose byte size isn’t a multiple of its element type size, then re-reads it to ensure it doesn’t fail (issue #9287).

Suggested change
return size() / typeSize;
return elements_.size();

Copilot uses AI. Check for mistakes.
@kevinbackhouse
Copy link
Copy Markdown
Collaborator

I checked the code history and it seems that we've been doing round-to-nearest here for a very long time, but I can't see any reason why. The call to std::lround was written by @neheb in #2768, but that was just to replace some older code that did the same thing in a less clean way.

If the tests pass, then I think this change is ok.

@neheb neheb merged commit 8d12298 into Exiv2:main Apr 6, 2026
103 of 110 checks passed
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.

TiffBinaryArray::doCount() uses floating-point rounding instead of integer division

4 participants