Fix ptsname() for big-endian architectures by pmorjan · Pull Request #20 · containerd/console

pmorjan · 2018-02-19T09:35:59Z

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 peter.morjan@de.ibm.com

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 <peter.morjan@de.ibm.com>

estesp · 2018-02-19T23:42:15Z

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 · 2018-02-20T16:10:47Z

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 · 2018-02-20T16:11:03Z

By pin, i mean 1.9.3 and 1.10/tip

pmorjan · 2018-02-20T16:20:11Z

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 · 2018-02-20T16:21:46Z

Works for me. Maybe allow failures only for 1.9.4?

pmorjan · 2018-02-20T16:34:41Z

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 · 2018-02-20T16:37:04Z

Ok, that works for me. Thanks for debugging

Solaris builds fail when using Go 1.9.4. Signed-off-by: Peter Morjan <peter.morjan@de.ibm.com>

crosbymichael · 2018-02-20T17:34:56Z

LGTM

estesp

LGTM

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 <afrosi@de.ibm.com>

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 <afrosi@de.ibm.com>

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 <afrosi@de.ibm.com>

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 <afrosi@de.ibm.com>

allow Travis CI to fail on Go 1.9.4

73a661c

Solaris builds fail when using Go 1.9.4. Signed-off-by: Peter Morjan <peter.morjan@de.ibm.com>

estesp approved these changes Feb 20, 2018

View reviewed changes

estesp merged commit 2748ece into containerd:master Feb 20, 2018

utzb mentioned this pull request Feb 22, 2018

docker exec -ti fails when there is already a TTY open for that container docker/for-linux#238

Closed

3 tasks

alicefr mentioned this pull request Nov 14, 2018

Update github.com/containerd/console in vendor kata-containers/agent#410

Closed

alicefr mentioned this pull request Nov 14, 2018

Update vendor/github.com/containerd/console kata-containers/agent#411

Merged

alicefr mentioned this pull request Nov 19, 2018

cli: Fix console for big endian architecture kata-containers/runtime#921

Closed

timotto mentioned this pull request Dec 5, 2020

Fix ptsname() for big-endian architectures pkg/term#59

Merged

kolyshkin mentioned this pull request Apr 6, 2021

terminal is broken on s390 (regression in rc93) opencontainers/runc#2896

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix ptsname() for big-endian architectures#20

Fix ptsname() for big-endian architectures#20
estesp merged 2 commits intocontainerd:masterfrom
pmorjan:fix-ptsname2

pmorjan commented Feb 19, 2018

Uh oh!

estesp commented Feb 19, 2018 •

edited

Loading

Uh oh!

crosbymichael commented Feb 20, 2018

Uh oh!

crosbymichael commented Feb 20, 2018

Uh oh!

pmorjan commented Feb 20, 2018

Uh oh!

crosbymichael commented Feb 20, 2018

Uh oh!

pmorjan commented Feb 20, 2018

Uh oh!

crosbymichael commented Feb 20, 2018

Uh oh!

crosbymichael commented Feb 20, 2018

Uh oh!

estesp left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

pmorjan commented Feb 19, 2018

Uh oh!

estesp commented Feb 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

crosbymichael commented Feb 20, 2018

Uh oh!

crosbymichael commented Feb 20, 2018

Uh oh!

pmorjan commented Feb 20, 2018

Uh oh!

crosbymichael commented Feb 20, 2018

Uh oh!

pmorjan commented Feb 20, 2018

Uh oh!

crosbymichael commented Feb 20, 2018

Uh oh!

crosbymichael commented Feb 20, 2018

Uh oh!

estesp left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

estesp commented Feb 19, 2018 •

edited

Loading