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

Jump to bottom

x/sys/unix: add portable speed accessor methods to Termios #18866

Closed
smithwinston opened this issue Jan 31, 2017 · 7 comments
Closed

x/sys/unix: add portable speed accessor methods to Termios #18866

smithwinston opened this issue Jan 31, 2017 · 7 comments
Labels
FrozenDueToAge
Milestone
Unreleased

Comments

@smithwinston
Copy link

Attempting to use the Ispeed and Ospeed members of syscall.Termios on mipsle fails with a compile error using go1.8rc3, compiling on Darwin:

bash-3.2$ export GOOS=linux GOARCH=mipsle
bash-3.2$ go1.8rc3 build termios.go
# command-line-arguments
./termios.go:10: unknown field 'Ispeed' in struct literal of type syscall.Termios
./termios.go:11: unknown field 'Ospeed' in struct literal of type syscall.Termios

Using this example program:

package main

import (
	"fmt"
	"syscall"
)

func main() {
	t := &syscall.Termios{
		Ispeed: 115200,
		Ospeed: 115200,
	}

	fmt.Printf("%#v\n", t)
}

However compilation is successful for arm, ppc64le and darwin using the same toolchain.

If I comment out the Ispeed and Ospeed initializers compile and run on a mipsle platform (an Omega2 from Onion), the program produces the following output:

&syscall.Termios{Iflag:0x0, Oflag:0x0, Cflag:0x0, Lflag:0x0, Line:0x0, Cc:[32]uint8{0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, Pad_cgo_0:[3]uint8{0x0, 0x0, 0x0}}

Compared to this on darwin:

&syscall.Termios{Iflag:0x0, Oflag:0x0, Cflag:0x0, Lflag:0x0, Cc:[20]uint8{0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, Pad_cgo_0:[4]uint8{0x0, 0x0, 0x0, 0x0}, Ispeed:0x1c200, Ospeed:0x1c200}

For reference, this is the output of go1.8rc3 env:

GOARCH="mipsle"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="linux"
GOPATH="/Users/wsmith/golang"
GORACE=""
GOROOT="/Users/wsmith/sdk/go1.8rc3"
GOTOOLDIR="/Users/wsmith/sdk/go1.8rc3/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
GOGCCFLAGS="-fPIC -mabi=32 -march=mips32 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/1j/_g9fzmm10vg7c10647c58wcm0000gn/T/go-build637432098=/tmp/go-build -gno-record-gcc-switches"
CXX="clang++"
CGO_ENABLED="0"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
@bradfitz
Copy link
Contributor

/cc @cherrymui @ianlancetaylor

@bradfitz bradfitz modified the milestones: Go1.4.4, Go1.8Maybe Jan 31, 2017
@cherrymui
Copy link
Member

syscall.Termios struct is generated from C header and matches the C type. It may vary on different platforms. In particular, on MIPS, the C type looks

struct termios
  {
    tcflag_t c_iflag;           /* input mode flags */
    tcflag_t c_oflag;           /* output mode flags */
    tcflag_t c_cflag;           /* control mode flags */
    tcflag_t c_lflag;           /* local mode flags */
    cc_t c_line;                /* line discipline */
    cc_t c_cc[NCCS];            /* control characters */
  };

I think your code should use build tags if you want to set these fields.

@vstefanovic
Copy link
Member

vstefanovic commented Jan 31, 2017 •
edited

These two fields aren't standard. An excerpt from http://man7.org/linux/man-pages/man3/termios.3.html:

POSIX says that the baud speed is stored in the termios
structure without specifying where precisely, and provides
cfgetispeed() and cfsetispeed() for getting at it.  Some
systems use bits selected by CBAUD in c_cflag, other systems
use separate fields, for example, sg_ispeed and sg_ospeed.

Looking at glibc's sysdeps/unix/sysv/linux/speed.c, you might want to use something like:

const (
	CBAUD   = uint32(0x100F)
	CBAUDEX = uint32(0x1000)
	IBAUD0  = uint32(0x80000000)
)

func SetIspeed(t *syscall.Termios,speed uint32) {
	if speed == 0 {
		t.Iflag |= IBAUD0
	} else {
		t.Iflag &= ^IBAUD0;
		t.Cflag &= ^(CBAUD | CBAUDEX);
		t.Cflag |= speed;
	}
}

func SetOspeed(t *syscall.Termios, speed uint32) {
	t.Cflag &= ^(CBAUD | CBAUDEX);
	t.Cflag |= speed;
}

(didn't see Cherry's comment.)

@ianlancetaylor ianlancetaylor changed the title Missing fields from syscall.Termios on mipsle [go1.8rc3] syscall: missing fields from Termios on mipsle [go1.8rc3] Jan 31, 2017
@ianlancetaylor
Copy link
Contributor

I don't think there is much we can do here. The structs really are different on different architectures. Changing the structs would mean that they would no longer work with the relevant ioctls.

In the golang.org/x/sys repo we could add accessor methods for these fields. That is probably our best available approach going forward. I'll repurpose this issue for that.

@ianlancetaylor ianlancetaylor changed the title syscall: missing fields from Termios on mipsle [go1.8rc3] x/sys/unix: add portable speed accessor methods to Termios Jan 31, 2017
@ianlancetaylor ianlancetaylor modified the milestones: Unreleased, Go1.8Maybe Jan 31, 2017
@smithwinston
Copy link
Author

@cherrymui, @vstefanovic, @ianlancetaylor thank you for your prompt and useful feedback. I didn't realize the intricacies of POSIX termios; this is actually the first platform I've had issues with! But I now have enough to move on! Thanks.

@tarm tarm mentioned this issue May 29, 2017
Closed
@tklauser
Copy link
Member

tklauser commented Aug 4, 2017 •
edited by bradfitz

In current master I see:

% git grep Ispeed ztypes_linux_*
ztypes_linux_386.go:691:        Ispeed uint32
ztypes_linux_amd64.go:710:      Ispeed uint32
ztypes_linux_arm.go:680:        Ispeed uint32
ztypes_linux_arm64.go:689:      Ispeed uint32
ztypes_linux_mips.go:685:       Ispeed uint32
ztypes_linux_mips64.go:691:     Ispeed uint32
ztypes_linux_mips64le.go:691:   Ispeed uint32
ztypes_linux_mipsle.go:685:     Ispeed uint32
ztypes_linux_ppc64.go:699:      Ispeed uint32
ztypes_linux_ppc64le.go:699:    Ispeed uint32
ztypes_linux_s390x.go:716:      Ispeed uint32
ztypes_linux_sparc64.go:664:    Ispeed uint32

(same for Ospeed)

They were added by https://golang.org/cl/17185, but only for 386, amd64, arm, arm64, ppc64 and ppc64le. When https://golang.org/cl/37943 regenerated all go files from unified sources, they were added
for mips*, s390x and sparc64 as well.

@bradfitz
Copy link
Contributor

bradfitz commented Aug 4, 2017

Sounds like the answer is to use golang.org/x/sys/unix then.

Closing this, but let me know if that doesn't work.

@bradfitz bradfitz closed this as completed Aug 4, 2017
@hnw hnw mentioned this issue Dec 14, 2017
Merged
@golang golang locked and limited conversation to collaborators Aug 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
7 participants
@bradfitz @tklauser @smithwinston @ianlancetaylor @gopherbot @cherrymui @vstefanovic