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

Download file to memory fix #20

Merged
merged 2 commits into from Sep 1, 2016

Conversation

iainlane
Copy link
Collaborator

@iainlane iainlane commented Sep 1, 2016

I was using make fast, and noticed that getFileContents wasn't working. I suspect that at higher optimisation levels the scope (exit) fclose (f) is happening earlier. It needs to happen before we read from the pointer (see the full commit message).

(Also squash some overly verbose debug logging, which isn't really related to this bug, but here we are.)

Iain Lane added 2 commits September 1, 2016 11:56
The documentation says

  The locations pointed to by ptr and sizeloc are used to report,
  respectively, the current location and the size of the buffer.  The
  locations referred to by these  pointers  are  updated each time the
  stream is flushed (fflush(3)) and when the stream is closed
  (fclose(3)).

so we need to close it before reading from the pointer. To do this we
move the memstream handling into the `if` block where it is needed.
@ximion ximion merged commit ecc817c into ximion:master Sep 1, 2016
@iainlane
Copy link
Collaborator Author

iainlane commented Sep 2, 2016

I guess you figured it out, but the answer is that you have to fclose before reading from the pointer.

@iainlane iainlane deleted the download-file-to-memory-fix branch September 2, 2016 14:03
@ximion
Copy link
Owner

ximion commented Sep 2, 2016

Yes, but you could just drop the scope(exit) and simply close the thing explicitly ;-)
But that's probably just a matter of taste.

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.

None yet

2 participants