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 ptsname() for big-endian architectures #20

Merged
merged 2 commits into from
Feb 20, 2018
Merged

Fix ptsname() for big-endian architectures #20

merged 2 commits into from
Feb 20, 2018

Conversation

pmorjan
Copy link
Contributor

@pmorjan pmorjan commented Feb 19, 2018

On big-endian architectures unix.IoctlGetInt() leads to a wrong result
because a 32 bit value is stored into a 64 bit buffer. When dereferencing
the result is left shifted by 32. Without this patch ptsname() returns
a wrong path from the second pty onwards.
To protect syscalls against re-arranging by the GC the conversion from
unsafe.Pointer to uintptr must occur in the Syscall expression itself.
See the documentation of the unsafe package for details.

Signed-off-by: Peter Morjan [email protected]

On big-endian architectures unix.IoctlGetInt() leads to a wrong result
because a 32 bit value is stored into a 64 bit buffer. When dereferencing
the result is left shifted by 32. Without this patch ptsname() returns
a wrong path from the second pty onwards.
To protect syscalls against re-arranging by the GC the conversion from
unsafe.Pointer to uintptr must occur in the Syscall expression itself.
See the documentation of the unsafe package for details.

Signed-off-by: Peter Morjan <[email protected]>
@estesp
Copy link
Member

estesp commented Feb 19, 2018

The Solaris build break with Go 1.9.4 is a known issue** (fixed in 1.10 which is why travis "tip" succeeds).

Should we: a) fix Golang 1.9.x to 1.9.3 in travis, b) move travis to 1.10, or c) remove Solaris from travis unless we have known use?

** hashicorp/consul#3879

@crosbymichael
Copy link
Member

Maybe pin to 1.9.3 so we don't lose solaris builds. I know we have some solaris users of this package so it would be nice to keep the build.

@crosbymichael
Copy link
Member

By pin, i mean 1.9.3 and 1.10/tip

@pmorjan
Copy link
Contributor Author

pmorjan commented Feb 20, 2018

You can allow Travis to fail for a particular Go version:
https://travis-ci.org/pmorjan/console
https://github.com/pmorjan/console/blob/travis/.travis.yml

language: go
go:
  - 1.9.3
  - 1.9.x
  - tip

go_import_path: github.com/containerd/console

install:
  - go get -d
  - GOOS=windows go get -d
  - GOOS=solaris go get -d

script:
  - go test -race
  - GOOS=windows go test
  - GOOS=solaris go build
  - GOOS=solaris go test -c

matrix:
  allow_failures:
    - go: 1.9.x

@crosbymichael
Copy link
Member

Works for me. Maybe allow failures only for 1.9.4?

@pmorjan
Copy link
Contributor Author

pmorjan commented Feb 20, 2018

but then you can't use the 1.9.x glob any more

this works:
========
go:
  - 1.9.3
  - 1.9.4
  - tip
matrix:
  allow_failures:
    - go: 1.9.4

this doesn't work
==========
go:
  - 1.9.x
  - 1.9.4
  - tip
matrix:
  allow_failures:
    - go: 1.9.4

@crosbymichael
Copy link
Member

Ok, that works for me. Thanks for debugging

Solaris builds fail when using Go 1.9.4.

Signed-off-by: Peter Morjan <[email protected]>
@crosbymichael
Copy link
Member

LGTM

Copy link
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

@estesp estesp merged commit 2748ece into containerd:master Feb 20, 2018
alicefr pushed a commit to alicefr/agent that referenced this pull request Nov 14, 2018
This PR fixes the issue kata-containers#410

The console package in vendor contains a big endian bug. The issue has
already been solved in runc. See PRs:
 - opencontainers/runc#1727
 - containerd/console#20

The console version has been updated from commit 84eeaae905 to commit
2748ece166.

The console package has been add in the Gopkg.toml because the console
package is not directly used in the code and the dep command doesn't
automatically update the console package.

Signed-off-by: Alice Frosi <[email protected]>
alicefr pushed a commit to alicefr/agent that referenced this pull request Nov 14, 2018
This PR fixes the issue kata-containers#410

The console package in vendor contains a big endian bug. The issue has
already been solved in runc. See PRs:
 - opencontainers/runc#1727
 - containerd/console#20

The console version has been updated from commit 84eeaae905 to commit
2748ece166.

The console package has been add in the Gopkg.toml because the console
package is not directly used in the code and the dep command doesn't
automatically update the console package.

Signed-off-by: Alice Frosi <[email protected]>
alicefr pushed a commit to alicefr/agent that referenced this pull request Nov 14, 2018
This PR fixes the issue kata-containers#410

The console package in vendor contains a big endian bug. The issue has
already been solved in runc. See PRs:
 - opencontainers/runc#1727
 - containerd/console#20

The console version has been updated from commit 84eeaae905 to commit
2748ece166 (commit that fixed the big-endian issue)

The console package has been added in the Gopkg.toml because the console
package is not directly used in the code and the dep command doesn't
automatically update the console package.

Signed-off-by: Alice Frosi <[email protected]>
alicefr pushed a commit to alicefr/agent that referenced this pull request Nov 15, 2018
The console package in vendor contains a big endian bug. The issue has
already been solved in runc. See PRs:
 - opencontainers/runc#1727
 - containerd/console#20

The console version has been updated from commit 84eeaae905 to commit
2748ece166 (commit that fixed the big-endian issue)

The console package has been added in the Gopkg.toml because the console
package is not directly used in the code and the dep command doesn't
automatically update the console package.

Signed-off-by: Alice Frosi <[email protected]>
alicefr pushed a commit to alicefr/agent that referenced this pull request Nov 15, 2018
Fixes: kata-containers#410

The console package in vendor contains a big endian bug. The issue has
already been solved in runc. See PRs:
 - opencontainers/runc#1727
 - containerd/console#20

The console version has been updated from commit 84eeaae905 to commit
2748ece166 (commit that fixed the big-endian issue)

The console package has been added in the Gopkg.toml because the console
package is not directly used in the code and the dep command doesn't
automatically update the console package.

Signed-off-by: Alice Frosi <[email protected]>
alicefr pushed a commit to alicefr/agent that referenced this pull request Nov 15, 2018
Fixes: kata-containers#410

The console package in vendor contains a big endian bug. The issue has
already been solved in runc. See PRs:
 - opencontainers/runc#1727
 - containerd/console#20

The console version has been updated from commit 84eeaae905 to commit
2748ece166 (commit that fixed the big-endian issue)

The console package has been added in the Gopkg.toml because the console
package is not directly used in the code and the dep command doesn't
automatically update the console package.

Signed-off-by: Alice Frosi <[email protected]>
alicefr pushed a commit to alicefr/agent that referenced this pull request Nov 15, 2018
Fixes: kata-containers#410

The console package in vendor contains a big endian bug. The issue has
already been solved in runc. See PRs:
 - opencontainers/runc#1727
 - containerd/console#20

The console version has been updated from commit 84eeaae905 to commit
2748ece166 (commit that fixed the big-endian issue)

The console package has been added in the Gopkg.toml because the console
package is not directly used in the code and the dep command doesn't
automatically update the console package.

Signed-off-by: Alice Frosi <[email protected]>
jshachm pushed a commit to jshachm/agent that referenced this pull request Nov 22, 2018
Fixes: kata-containers#410

The console package in vendor contains a big endian bug. The issue has
already been solved in runc. See PRs:
 - opencontainers/runc#1727
 - containerd/console#20

The console version has been updated from commit 84eeaae905 to commit
2748ece166 (commit that fixed the big-endian issue)

The console package has been added in the Gopkg.toml because the console
package is not directly used in the code and the dep command doesn't
automatically update the console package.

Signed-off-by: Alice Frosi <[email protected]>
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