Skip to content

Conversation

@eldridgemartinez
Copy link

Please refer to issue #303 for complete replication

I certify that I own, and have sufficient rights to contribute, all source code and related material intended to be compiled or integrated with the source code for the SharpZipLib open source product (the "Contribution"). My Contribution is licensed under the MIT License.

@eldridgemartinez eldridgemartinez changed the title read more byte to fill the required length read more bytes to fill the required length Jan 15, 2019
if (obtained > 0 && obtained < lengthToRead)
{
// less bytes are returned but may not be the end of file. Get more to fill the required length. Eof returns 0 bytes so this is safe
int moreObtained = _stream.Read(_slideBuffer, _slideBufFreePos, lengthToRead - obtained);

Choose a reason for hiding this comment

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

Why only 1 extra read? You could try reading from the inner stream until the buffer is completely filled or the inner stream returns 0 bytes. Seems to be more solid.

Copy link
Author

Choose a reason for hiding this comment

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

It needs one extra read to fill the required length else it throws error and I don't want to change the logic of lengthToRead. This one works and I tested it. Can you help me how to fix this?

Copy link
Contributor

Choose a reason for hiding this comment

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

trying to handle the case where the second read returns to little data as well seems reasonable. Would something like Numpsy@c328b3c be reasonable?

Copy link
Author

Choose a reason for hiding this comment

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

Yes thats good. I will cancel my PR if that is working and merged. Let me know if there is a new nuget release. Thank you

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.

3 participants