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

Fix buffered seek #118

Merged
merged 3 commits into from
Nov 18, 2016
Merged

Fix buffered seek #118

merged 3 commits into from
Nov 18, 2016

Conversation

yeswalrus
Copy link
Contributor

BufferedStream did not implement Seek. This function is vital to some downstream functionality.

@jdonald
Copy link
Contributor

jdonald commented Nov 17, 2016

When I built this via the ExternalBuilder Jenkins task, compilation errors seem to be due to missing the <stdexcept> header.

@yeswalrus
Copy link
Contributor Author

try now?

ASSERT_EQ(0, bs.Read(buf, sizeof(buf)));

}

Copy link
Contributor

Choose a reason for hiding this comment

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

Two extraneous newlines

m_readOffset = std::min<std::streamoff>(std::max<std::streamoff>(0, pos), m_lastValidByte);
m_eof = m_readOffset != pos && m_readOffset > 0;
return this;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

missing newline at EOF

@@ -42,5 +42,6 @@ namespace leap {
std::streamsize Read(void* pBuf, std::streamsize ncb) override;
std::streamsize Skip(std::streamsize ncb) override;
std::streamsize Length(void) override;
IInputStream* Seek(std::streampos off) override;
};
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Here too although it was this way before your change.

@yeswalrus
Copy link
Contributor Author

We should add an automated check for those formatting issues...

@jdonald
Copy link
Contributor

jdonald commented Nov 18, 2016

I finally managed to run LeapSerialTest on Android for both 32-bit and 64-bit via my tmp branch to verify this. Since we lack Android coverage in Jenkins, it broke some time ago so I'll fix that separately.

@jdonald jdonald merged commit 7b280b2 into master Nov 18, 2016
@jdonald jdonald deleted the fix-buffered-seek branch November 18, 2016 03:37
@yeswalrus yeswalrus added this to the v0.5.0 milestone May 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants