many: fix review comments from PR #3177 #3244

Merged
merged 4 commits into from Apr 27, 2017

Conversation

Projects
None yet
5 participants
Contributor

morphis commented Apr 26, 2017

Based on feedback from @niemeyer on #3177

@mvo5 mvo5 added this to the 2.25 milestone Apr 26, 2017

mvo5 approved these changes Apr 26, 2017

Thanks for doing this work!

x11/xauth.go
// A chunk consists of a length encoded by two bytes and
// additional data which is the real value of the item
// we reading here from the file.
b := [2]byte{}
- if err := readBytes(f, b[:]); err != nil {
+ if _, err := io.ReadFull(r, b[:]); err != nil {
@mvo5

mvo5 Apr 26, 2017

Collaborator

Nice!

zyga approved these changes Apr 27, 2017

Looks good

lgtm

two things

osutil/cmp.go
@@ -63,12 +63,12 @@ func FilesAreEqual(a, b string) bool {
// FileStreamsEqual compares two file streams and returns true if both
// have the same content.
-func FileStreamsEqual(fa, fb io.Reader) bool {
+func FileStreamsEqual(a, b io.Reader) bool {
@pedronis

pedronis Apr 27, 2017

Contributor

s/File// if I understand the original comments correctly

x11/xauth.go
-}
-
-func readChunk(f *os.File) ([]byte, error) {
+func readChunk(r io.Reader) ([]byte, error) {
// A chunk consists of a length encoded by two bytes and
@pedronis

pedronis Apr 27, 2017

Contributor

... two bytes (so max 64K) ...

Simon Fels added some commits Apr 26, 2017

Contributor

niemeyer commented Apr 27, 2017

Thank you!

@niemeyer niemeyer merged commit 691c82c into snapcore:master Apr 27, 2017

2 of 6 checks passed

xenial-amd64 autopkgtest finished (failure)
Details
xenial-i386 autopkgtest finished (failure)
Details
yakkety-amd64 autopkgtest finished (failure)
Details
zesty-amd64 autopkgtest finished (failure)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
xenial-ppc64el autopkgtest finished (success)
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment