golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.14k stars 17.69k forks source link

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

Closed smithwinston closed 7 years ago

smithwinston commented 7 years ago

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 commented 7 years ago

/cc @cherrymui @ianlancetaylor

cherrymui commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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.

smithwinston commented 7 years ago

@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.

tklauser commented 7 years ago

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 commented 7 years ago

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

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