We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 1696 - New FileDescriptor based RWops system is broken
Summary: New FileDescriptor based RWops system is broken
Status: RESOLVED FIXED
Alias: None
Product: SDL
Classification: Unclassified
Component: file (show other bugs)
Version: HG 2.0
Hardware: ARM Android (All)
: P2 major
Assignee: Sam Lantinga
QA Contact: Sam Lantinga
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-14 02:16 UTC by paul
Modified: 2013-01-27 16:44 UTC (History)
2 users (show)

See Also:

Attachments
Patch to fix the issue. (603 bytes, application/octet-stream)
2013-01-14 02:16 UTC, paul 		Details
Patch for not deleted global reference on Android. (1.16 KB, patch)
2013-01-27 05:46 UTC, Philipp Wiesemann 		Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description paul 2013-01-14 02:16:17 UTC 
Created attachment 1023 [details]
Patch to fix the issue.

The newly added Android RWops system for directly accessing the native filehandle
does not seek to the correct offset in the file. 

So if you don't do a seek before reading data, you won't get what you expect.

I have attached a patch that fixes the issue for me.
Comment 1 Gabriel Jacobo 2013-01-14 15:21:29 UTC 
Thanks! http://hg.libsdl.org/SDL/rev/b3d3ef1e15b5
Comment 2 Philipp Wiesemann 2013-01-16 12:37:31 UTC 
There is maybe another problem: The GlobalRef stored in assetFileDescriptorRef is not deleted if the control flow does "goto fallback".  It is only deleted if it does "goto failure". This may be fixed if the GlobalRef is created later (after gotos).
Comment 3 Philipp Wiesemann 2013-01-27 05:46:56 UTC 
Created attachment 1029 [details]
Patch for not deleted global reference on Android.

I attached a patch for the problem I tried to describe above (global reference not deleted on error).

I do not know how often exception might happen at "getStartOffset" and "getDeclaredLength" but I think there was a reason the checks were added. Without the described change this part of the source will not be "exception safe" because it leaks.

On the other hand if no exceptions might happen there at all the checks and gotos might be removed to make the source more clear. The the creation of the global reference does not need to be changed.
Comment 4 Gabriel Jacobo 2013-01-27 16:44:08 UTC 
http://hg.libsdl.org/SDL/rev/ac7f004fb63c Thanks!