BurntSushi / xgb

The X Go Binding is a low-level API to communicate with the X server. It is modeled on XCB and supports many X extensions.
Other
489 stars 75 forks source link

DISPLAY=:0.1 with just one X11 "screen" makes xproto.Setup(...).DefaultScreen panic #23

Open tv42 opened 10 years ago

tv42 commented 10 years ago

It seems setup.DefaultScreen does not check before assuming that DISPLAY is valid.

    X, err := xgb.NewConn()
    if err != nil {
        log.Fatal(err)
    }

    setup := xproto.Setup(X)
    screen := setup.DefaultScreen(X)
    fmt.Printf("%dx%d\n", screen.WidthInPixels, screen.HeightInPixels)
$ DISPLAY=:0.1 x11-screen-pixels 
panic: runtime error: index out of range

goroutine 16 [running]:
runtime.panic(0x593140, 0x6a9c1c)
    /home/tv/src/go/src/pkg/runtime/panic.c:279 +0xf5
main.main()
    /home/tv/go/src/github.com/tv42/x11-screen-pixels/main.go:42 +0x2eb

goroutine 19 [finalizer wait]:
runtime.park(0x413890, 0x6ad358, 0x6abec9)
    /home/tv/src/go/src/pkg/runtime/proc.c:1369 +0x89
runtime.parkunlock(0x6ad358, 0x6abec9)
    /home/tv/src/go/src/pkg/runtime/proc.c:1385 +0x3b
runfinq()
    /home/tv/src/go/src/pkg/runtime/mgc0.c:2644 +0xcf
runtime.goexit()
    /home/tv/src/go/src/pkg/runtime/proc.c:1445

goroutine 20 [runnable]:
github.com/BurntSushi/xgb.(*Conn).generateXIds(0xc2080580a0)
    /home/tv/go/src/github.com/BurntSushi/xgb/xgb.go:200
created by github.com/BurntSushi/xgb.NewConnDisplay
    /home/tv/go/src/github.com/BurntSushi/xgb/xgb.go:110 +0x1bb

goroutine 21 [runnable]:
github.com/BurntSushi/xgb.(*Conn).generateSeqIds(0xc2080580a0)
    /home/tv/go/src/github.com/BurntSushi/xgb/xgb.go:253
created by github.com/BurntSushi/xgb.NewConnDisplay
    /home/tv/go/src/github.com/BurntSushi/xgb/xgb.go:111 +0x1d3

goroutine 22 [runnable]:
github.com/BurntSushi/xgb.(*Conn).sendRequests(0xc2080580a0)
    /home/tv/go/src/github.com/BurntSushi/xgb/xgb.go:300
created by github.com/BurntSushi/xgb.NewConnDisplay
    /home/tv/go/src/github.com/BurntSushi/xgb/xgb.go:112 +0x1eb

goroutine 23 [runnable]:
github.com/BurntSushi/xgb.(*Conn).readResponses(0xc2080580a0)
    /home/tv/go/src/github.com/BurntSushi/xgb/xgb.go:349
created by github.com/BurntSushi/xgb.NewConnDisplay
    /home/tv/go/src/github.com/BurntSushi/xgb/xgb.go:113 +0x203
BurntSushi commented 10 years ago

That's fair. What is the expected behavior?

tv42 commented 10 years ago

Good question! The only place that currently returns errors is NewConn, which may be too early. Maybe DefaultScreen needs an error return?

BurntSushi commented 10 years ago

That would be a breaking change, unfortunately. I haven't really implemented any versioning scheme yet, but XGB has been stable for long enough that I really don't want to introduce any breaking changes without really good reason.

tv42 commented 10 years ago

Validate it in Conn.connect? I don't know if that would add round-trips.