Skip to content
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.

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

Inveon: fix offsets for files larger than 2GB #1502

Merged
merged 1 commit into from Feb 17, 2015

Conversation

melissalinkert
Copy link
Member

See
http://lists.openmicroscopy.org.uk/pipermail/ome-users/2014-December/004972.html

Builds should remain green with this change; the only thing to verify is that the offset passed to dat.seek is calculated using long (not int) arithmetic.

@@ -110,7 +110,7 @@ public boolean isThisType(RandomAccessInputStream stream) throws IOException {
{
FormatTools.checkPlaneParameters(this, no, buf.length, x, y, w, h);

int planeSize = FormatTools.getPlaneSize(this);
long planeSize = (long) FormatTools.getPlaneSize(this);
Copy link

Choose a reason for hiding this comment

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

Given that FormatTools.getPlaneSize() is returning an int and sqrt(2^31) => 46340 we're likely to overflow for big histopathology images in general. Should getPlaneSize (and the other helpers which use it) all be switched to use long, saving the need for the cast here? This will ensure we're not routinely overflowing for all readers using this and related helpers.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's up for debate I guess, but outside the scope of this PR. Either way the return value of getPlaneSize(this) would need to be cast - if it returns long we're just casting when a byte array is constructed instead.

Copy link

Choose a reason for hiding this comment

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

Yes, it's certainly outside the scope of this PR, just thought it worth mentioning if it's a more general limitation we might want to address as a follow-on task.

melissalinkert added a commit that referenced this pull request Feb 17, 2015
Inveon: fix offsets for files larger than 2GB
@melissalinkert melissalinkert merged commit d6e7205 into ome:develop Feb 17, 2015
@melissalinkert
Copy link
Member Author

--no-rebase

@sbesson sbesson added this to the 5.1.0-m4 milestone Feb 20, 2015
@melissalinkert melissalinkert deleted the inveon-offsets branch April 13, 2015 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants