diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 00000000..af5b8126 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,76 @@ +run: + deadline: 10m + +issues: + exclude-rules: + + # Exclude `for i, _ :=`, since that doesn't work in old go + - linters: + - gosimple + text: "should omit value from range" + - linters: + - golint + text: "should omit 2nd value from range" + + # Exclude underscore warnings, too much work + # for a semi-vendored wrapper + - linters: + - golint + path: usb/lowlevel/libusb/libusb.go + text: "don't use underscores" + + # Exclude all cap warnings, too much work + # for a semi-vendored wrapper + - linters: + - golint + path: usb/lowlevel/libusb/libusb.go + text: "ALL_CAPS" + + # Exclude long lines warnings, too much work + # for a semi-vendored wrapper + - linters: + - lll + path: usb/lowlevel/libusb/libusb.go + + # Exclude noinit warnings, too much work + # for a semi-vendored wrapper + - linters: + - lll + path: usb/lowlevel/hidapi/hid.go + + # Before we split into internal/non-internal, ignore comments + - linters: + - golint + text: "(comment on exported (method|function|type|const)|should have( a package)? comment|comment should be of the form)" + + exclude-use-default: false + +linters: + enable: + - golint + - govet + - staticcheck + - gosec + - stylecheck + - interfacer + - unconvert + - dupl + - goconst + - gocyclo + - gofmt + - goimports + - maligned + - depguard + - misspell + - lll + - unparam + - nakedret + - prealloc + - scopelint + - gochecknoinits + - gocritic + +linters-settings: + gofmt: + # causes errors in old go + simplify: false diff --git a/.gometalinter b/.gometalinter deleted file mode 100644 index 5dc38fc6..00000000 --- a/.gometalinter +++ /dev/null @@ -1,9 +0,0 @@ -{ - "Deadline": "300s", - "Exclude": [ - "should have comment or be unexported", - "should have comment \\(or a comment on this block\\) or be unexported", - "http\\.CloseNotifier is deprecated" - ], - "Vendor": true -} diff --git a/.travis.yml b/.travis.yml index fa84ac93..020768a0 100644 --- a/.travis.yml +++ b/.travis.yml @@ -45,17 +45,10 @@ matrix: - go get github.com/karalabe/xgo - cd release/macos - make all - - env: TEST=go-vet + - env: TEST=golangci script: - - go vet ./... - - env: TEST=go-fmt - script: - - gofmt -d *.go ./server ./server/status ./server/api ./usb ./wire ./core ./memorywriter - - env: TEST=metalinter - script: - - go get -u github.com/alecthomas/gometalinter - - gometalinter --install - - gometalinter + - curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s -- -b $(go env GOPATH)/bin v1.15.0 + - golangci-lint run - env: TEST=version-check script: - diff -u VERSION <(grep "const version" trezord.go | cut -f 2 -d '"') diff --git a/core/core.go b/core/core.go index b849ec88..72545a4a 100644 --- a/core/core.go +++ b/core/core.go @@ -17,6 +17,8 @@ import ( "github.com/trezor/trezord-go/wire" ) +var ErrTimeout = errors.New("timeout") + // Package with "core logic" of device listing // and dealing with sessions, mutexes, ... // @@ -61,10 +63,23 @@ type USBInfo struct { } type USBDevice interface { - io.ReadWriter + io.Writer + Read(buf []byte, stopShortTimeout bool) (int, error) Close(disconnected bool) error } +type USBDeviceIoWriter struct { + device USBDevice +} + +func (u USBDeviceIoWriter) Write(buf []byte) (int, error) { + return u.device.Write(buf) +} + +func (u USBDeviceIoWriter) Read(buf []byte) (int, error) { + return u.device.Read(buf, false) +} + type session struct { path string id string @@ -130,6 +145,8 @@ type Core struct { log *memorywriter.MemoryWriter latestSessionID int + + devicesWithUnreadData map[string]bool // info.Path, not usbPath } var ( @@ -156,6 +173,8 @@ func New(bus USBBus, log *memorywriter.MemoryWriter, allowStealing, reset bool) allowStealing: allowStealing, reset: reset, usbPaths: make(map[int]string), + + devicesWithUnreadData: make(map[string]bool), } go c.backgroundListen() return c @@ -228,7 +247,7 @@ func (c *Core) saveUsbPaths(devs []USBInfo) (res []USBInfo) { Debug: dev.Debug, }) } - return + return res } func (c *Core) Enumerate() ([]EnumerateEntry, error) { @@ -261,26 +280,25 @@ func (c *Core) Enumerate() ([]EnumerateEntry, error) { c.lastInfos = infos } - entries := c.createEnumerateEntries(infos) c.log.Log("release disconnected") c.releaseDisconnected(infos, false) c.releaseDisconnected(infos, true) - return entries, nil -} -func (c *Core) findSession(e *EnumerateEntry, path string, debug bool) { - for _, ss := range c.sessions(debug) { - if ss.path == path { - // Copying to prevent overwriting on Acquire and - // wrong comparison in Listen. - ssidCopy := ss.id - if debug { - e.DebugSession = &ssidCopy - } else { - e.Session = &ssidCopy - } - } + // quickly check all the non-acquired devices, + // if there is not some dangling message (when closed during reading) + // + // It is done here, because + // (1) enumerate usually does run in the background because of backgroundListen + // (2) however, it does NOT run in the background if nobody "starts" it + // so it should not break 3rd party stuff that connects to device directly + // (3) at this point we can be sure nobody is really reading the device + if c.callsInProgress == 0 { + c.cleanupReadQueue(infos, false) + c.cleanupReadQueue(infos, true) } + + entries := c.createEnumerateEntries(infos) + return entries, nil } func (c *Core) createEnumerateEntry(info USBInfo) EnumerateEntry { @@ -291,8 +309,8 @@ func (c *Core) createEnumerateEntry(info USBInfo) EnumerateEntry { Type: info.Type, Debug: info.Debug, } - c.findSession(&e, info.Path, false) - c.findSession(&e, info.Path, true) + c.findWriteSession(&e, info.Path, false) + c.findWriteSession(&e, info.Path, true) return e } @@ -318,6 +336,60 @@ func (c *Core) sessions(debug bool) map[string]*session { return sessions } +func (c *Core) cleanupReadQueue(infos []USBInfo, debug bool) { + c.log.Log("start") + + // note - this is run after releaseDisconnected, + // we know that devices actually exist etc +INFOS: + for _, info := range infos { + if debug && !info.Debug { + continue INFOS + } + session := c.findSession(info.Path, debug) + if session != "" { + continue INFOS + } + if !c.devicesWithUnreadData[info.Path] { + continue INFOS + } + usbPath, err := c.getUSBPath(info.Path) + if err != nil { + c.log.Log(fmt.Sprintf("Error on cleanup: %s", err)) + continue INFOS + } + dev, err := c.tryConnect(usbPath, debug, false) + if err != nil { + c.log.Log(fmt.Sprintf("Error on cleanup: %s", err)) + continue INFOS + } + // note - do NOT need lock callMutex, since + // this already happens in Enumerate + err = nil + readOnce := false + var buf [64]byte + for err == nil { + _, err = dev.Read(buf[:], true) + if err != nil { + if readOnce { + delete(c.devicesWithUnreadData, info.Path) + } + c.log.Log("read err") + if err != ErrTimeout { + c.log.Log(fmt.Sprintf("Error on cleanup: %s", err)) + } + errR := dev.Close(false) + if errR != nil { + c.log.Log(fmt.Sprintf("Error on cleanup: %s", errR)) + } + continue INFOS + } + readOnce = true + c.log.Log("read") + } + } +} + func (c *Core) releaseDisconnected(infos []USBInfo, debug bool) { for ssid, ss := range c.sessions(debug) { @@ -328,6 +400,7 @@ func (c *Core) releaseDisconnected(infos []USBInfo, debug bool) { } } if !connected { + delete(c.devicesWithUnreadData, ss.path) c.log.Log(fmt.Sprintf("disconnected device %s", ssid)) err := c.release(ssid, true, debug) // just log if there is an error @@ -361,7 +434,7 @@ func (c *Core) release( return err } -func (c *Core) Listen(entries []EnumerateEntry, ctx context.Context) ([]EnumerateEntry, error) { +func (c *Core) Listen(ctx context.Context, entries []EnumerateEntry) ([]EnumerateEntry, error) { c.log.Log("start") EnumerateEntries(entries).Sort() @@ -394,7 +467,7 @@ func (c *Core) Listen(entries []EnumerateEntry, ctx context.Context) ([]Enumerat return entries, nil } -func (c *Core) findPrevSession(path string, debug bool) string { +func (c *Core) findSession(path string, debug bool) string { // note - sessionsMutex must be locked before entering this for _, ss := range c.sessions(debug) { if ss.path == path { @@ -405,6 +478,31 @@ func (c *Core) findPrevSession(path string, debug bool) string { return "" } +func (c *Core) findWriteSession(e *EnumerateEntry, path string, debug bool) { + ssid := c.findSession(path, debug) + if ssid != "" { + if debug { + e.DebugSession = &ssid + } else { + e.Session = &ssid + } + } +} + +func (c *Core) getUSBPath(path string) (string, error) { + + pathI, err := strconv.Atoi(path) + if err != nil { + return "", err + } + + usbPath, exists := c.usbPaths[pathI] + if !exists { + return "", errors.New("device not found") + } + return usbPath, nil +} + func (c *Core) Acquire( path, prev string, debug bool, @@ -419,7 +517,7 @@ func (c *Core) Acquire( c.log.Log(fmt.Sprintf("input path %s prev %s", path, prev)) - prevSession := c.findPrevSession(path, debug) + prevSession := c.findSession(path, debug) c.log.Log(fmt.Sprintf("actually previous %s", prevSession)) @@ -441,25 +539,22 @@ func (c *Core) Acquire( // reset device ONLY if no call on the other port // otherwise, USB reset stops other call - otherSession := c.findPrevSession(path, !debug) + otherSession := c.findSession(path, !debug) reset := otherSession == "" && c.reset - pathI, err := strconv.Atoi(path) + usbPath, err := c.getUSBPath(path) if err != nil { return "", err } - usbPath, exists := c.usbPaths[pathI] - if !exists { - return "", errors.New("device not found") - } - c.log.Log("trying to connect") dev, err := c.tryConnect(usbPath, debug, reset) if err != nil { return "", err } + delete(c.devicesWithUnreadData, path) + id := c.newSession(debug) sess := &session{ @@ -517,11 +612,11 @@ const ( ) func (c *Core) Call( + ctx context.Context, body []byte, session string, mode CallMode, debug bool, - ctx context.Context, ) ([]byte, error) { c.log.Log("callMutex lock") c.callMutex.Lock() @@ -584,9 +679,13 @@ func (c *Core) Call( }() c.log.Log("before actual logic") - bytes, err := c.readWriteDev(body, acquired.dev, mode) + bytes, err := c.readWriteDev(body, &USBDeviceIoWriter{device: acquired.dev}, mode) c.log.Log("after actual logic") + if err != nil { + c.devicesWithUnreadData[acquired.path] = true + c.log.Log("canceled") + } return bytes, err } diff --git a/server/api/api.go b/server/api/api.go index 45004869..569292d9 100644 --- a/server/api/api.go +++ b/server/api/api.go @@ -24,7 +24,7 @@ type api struct { logger *memorywriter.MemoryWriter } -func ServeAPI(r *mux.Router, c *core.Core, v string, l *memorywriter.MemoryWriter) error { +func ServeAPI(r *mux.Router, c *core.Core, v string, l *memorywriter.MemoryWriter) { api := &api{ core: c, version: v, @@ -46,12 +46,8 @@ func ServeAPI(r *mux.Router, c *core.Core, v string, l *memorywriter.MemoryWrite r.HandleFunc("/debug/call/{session}", api.CallDebug) r.HandleFunc("/debug/post/{session}", api.PostDebug) r.HandleFunc("/debug/read/{session}", api.ReadDebug) - corsv, err := corsValidator() - if err != nil { - return err - } + corsv := corsValidator() r.Use(CORS(corsv)) - return nil } func (a *api) Info(w http.ResponseWriter, r *http.Request) { @@ -86,7 +82,7 @@ func (a *api) Listen(w http.ResponseWriter, r *http.Request) { return } - res, err := a.core.Listen(entries, r.Context()) + res, err := a.core.Listen(r.Context(), entries) if err != nil { a.respondError(w, err) return @@ -210,7 +206,7 @@ func (a *api) call(w http.ResponseWriter, r *http.Request, mode core.CallMode, d } } - binres, err := a.core.Call(binbody, session, mode, debug, r.Context()) + binres, err := a.core.Call(r.Context(), binbody, session, mode, debug) if err != nil { a.respondError(w, err) return @@ -226,16 +222,10 @@ func (a *api) call(w http.ResponseWriter, r *http.Request, mode core.CallMode, d } } -func corsValidator() (OriginValidator, error) { - tregex, err := regexp.Compile(`^https://([[:alnum:]\-_]+\.)*trezor\.io$`) - if err != nil { - return nil, err - } +func corsValidator() OriginValidator { + tregex := regexp.MustCompile(`^https://([[:alnum:]\-_]+\.)*trezor\.io$`) // `localhost:8xxx` and `5xxx` are added for easing local development. - lregex, err := regexp.Compile(`^https?://localhost:[58][[:digit:]]{3}$`) - if err != nil { - return nil, err - } + lregex := regexp.MustCompile(`^https?://localhost:[58][[:digit:]]{3}$`) v := func(origin string) bool { if lregex.MatchString(origin) { return true @@ -254,7 +244,7 @@ func corsValidator() (OriginValidator, error) { return false } - return v, nil + return v } func (a *api) checkJSONError(w http.ResponseWriter, err error) { diff --git a/server/api/api_test.go b/server/api/api_test.go index fc05910f..3f18ac36 100644 --- a/server/api/api_test.go +++ b/server/api/api_test.go @@ -30,10 +30,7 @@ func TestOriginValidator(t *testing.T) { {"http://localhost", false}, {"http://localhost:1234", false}, } - validator, err := corsValidator() - if err != nil { - t.Fatal(err) - } + validator := corsValidator() for _, tc := range testcases { allow := validator(tc.origin) if allow != tc.allow { diff --git a/server/http.go b/server/http.go index 0f653860..7cb3652f 100644 --- a/server/http.go +++ b/server/http.go @@ -51,10 +51,7 @@ func New( redirectRouter := r.Methods("GET").Path("/").Subrouter() status.ServeStatus(statusRouter, c, version, shortWriter, longWriter) - err := api.ServeAPI(postRouter, c, version, longWriter) - if err != nil { - panic(err) // only error is an error from originValidator regexp constructor - } + api.ServeAPI(postRouter, c, version, longWriter) status.ServeStatusRedirect(redirectRouter) diff --git a/server/status/status.go b/server/status/status.go index b4a83677..a275c349 100644 --- a/server/status/status.go +++ b/server/status/status.go @@ -97,7 +97,13 @@ func (s *status) statusGzip(w http.ResponseWriter, r *http.Request) { return } - start := s.version + "\n" + msinfo + "\n" + devconLog + devconLogD + "\n" + old + libwdi + setupapi + "\nCurrent log:\n" + start := s.version + "\n" + + msinfo + "\n" + + devconLog + devconLogD + "\n" + + old + + libwdi + + setupapi + + "\nCurrent log:\n" gzip, err := s.longMemoryWriter.Gzip(start) if err != nil { diff --git a/server/status/template.go b/server/status/template.go index d6374c2a..6dd945c8 100644 --- a/server/status/template.go +++ b/server/status/template.go @@ -214,7 +214,9 @@ const templateString = ` {{if .IsWindows}}

- Detailed log might take a while to generate.
It might also reveal detailed information about your PC configuration. + Detailed log might take a while to generate. +
+ It might also reveal detailed information about your PC configuration.

{{end}} diff --git a/trezord.go b/trezord.go index e3c74b78..f7442976 100644 --- a/trezord.go +++ b/trezord.go @@ -25,7 +25,7 @@ func (i *udpTouples) String() string { res := "" for i, p := range *i { if i > 0 { - res = res + "," + res += "," } res = res + strconv.Itoa(p.Normal) + ":" + strconv.Itoa(p.Debug) } @@ -55,9 +55,9 @@ func (i *udpPorts) String() string { res := "" for i, p := range *i { if i > 0 { - res = res + "," + res += "," } - res = res + strconv.Itoa(p) + res += strconv.Itoa(p) } return res } @@ -103,13 +103,48 @@ func main() { var reset bool var versionFlag bool - flag.StringVar(&logfile, "l", "", "Log into a file, rotating after 20MB") - flag.Var(&ports, "e", "Use UDP port for emulator. Can be repeated for more ports. Example: trezord-go -e 21324 -e 21326") - flag.Var(&touples, "ed", "Use UDP port for emulator with debug link. Can be repeated for more ports. Example: trezord-go -ed 21324:21326") - flag.BoolVar(&withusb, "u", true, "Use USB devices. Can be disabled for testing environments. Example: trezord-go -e 21324 -u=false") - flag.BoolVar(&verbose, "v", false, "Write verbose logs to either stderr or logfile") - flag.BoolVar(&versionFlag, "version", false, "Write version") - flag.BoolVar(&reset, "r", true, "Reset USB device on session acquiring. Enabled by default (to prevent wrong device states); set to false if you plan to connect to debug link outside of bridge.") + flag.StringVar( + &logfile, + "l", + "", + "Log into a file, rotating after 20MB", + ) + flag.Var( + &ports, + "e", + "Use UDP port for emulator. Can be repeated for more ports. Example: trezord-go -e 21324 -e 21326", + ) + flag.Var( + &touples, + "ed", + "Use UDP port for emulator with debug link. Can be repeated for more ports. Example: trezord-go -ed 21324:21326", + ) + flag.BoolVar( + &withusb, + "u", + true, + "Use USB devices. Can be disabled for testing environments. Example: trezord-go -e 21324 -u=false", + ) + flag.BoolVar( + &verbose, + "v", + false, + "Write verbose logs to either stderr or logfile", + ) + flag.BoolVar( + &versionFlag, + "version", + false, + "Write version", + ) + flag.BoolVar( + &reset, + "r", + true, + "Reset USB device on session acquiring. "+ + "Enabled by default (to prevent wrong device states); "+ + "set to false if you plan to connect to debug link outside of bridge.", + ) flag.Parse() if versionFlag { diff --git a/usb/bus.go b/usb/bus.go index 17219335..7afeaa0b 100644 --- a/usb/bus.go +++ b/usb/bus.go @@ -26,7 +26,7 @@ func (b *USB) Has(path string) bool { } func (b *USB) Enumerate() ([]core.USBInfo, error) { - var infos []core.USBInfo + infos := make([]core.USBInfo, 0, len(b.buses)) for _, b := range b.buses { l, err := b.Enumerate() diff --git a/usb/hidapi.go b/usb/hidapi.go index c16cc68a..69f63001 100644 --- a/usb/hidapi.go +++ b/usb/hidapi.go @@ -29,6 +29,7 @@ type HIDAPI struct { } func InitHIDAPI(mw *memorywriter.MemoryWriter) (*HIDAPI, error) { + lowlevel.Init() lowlevel.SetLogWriter(mw) return &HIDAPI{ mw: mw, @@ -44,6 +45,7 @@ func (b *HIDAPI) Enumerate() ([]core.USBInfo, error) { b.mw.Log("low level done") for _, dev := range devs { // enumerate all devices + dev := dev // pin, see github.com/kyoh86/scopelint if b.match(&dev) { infos = append(infos, core.USBInfo{ Path: b.identify(&dev), @@ -70,6 +72,7 @@ func (b *HIDAPI) Connect(path string, debug bool, reset bool) (core.USBDevice, e b.mw.Log("enumerate done") for _, dev := range devs { // enumerate all devices + dev := dev // pin, see github.com/kyoh86/scopelint if b.match(&dev) && b.identify(&dev) == path { b.mw.Log("low level open") d, err := dev.Open() @@ -173,7 +176,7 @@ func (b *HIDAPI) detectPrepend(dev *lowlevel.HidDevice) (bool, error) { return false, errors.New("unknown HID version") } -func (d *HID) readWrite(buf []byte, read bool) (int, error) { +func (d *HID) readWrite(buf []byte, read bool, stopShortTimeout bool) (int, error) { d.mw.Log("start") for { @@ -214,6 +217,10 @@ func (d *HID) readWrite(buf []byte, read bool) (int, error) { return 0, errors.New("HID - empty write") } + if stopShortTimeout { + return 0, core.ErrTimeout + } + d.mw.Log("skipping empty transfer - go again") } else { if err.Error() == unknownErrorMessage { @@ -227,9 +234,9 @@ func (d *HID) readWrite(buf []byte, read bool) (int, error) { } func (d *HID) Write(buf []byte) (int, error) { - return d.readWrite(buf, false) + return d.readWrite(buf, false, false) } -func (d *HID) Read(buf []byte) (int, error) { - return d.readWrite(buf, true) +func (d *HID) Read(buf []byte, stopShortTimeout bool) (int, error) { + return d.readWrite(buf, true, stopShortTimeout) } diff --git a/usb/hidapi_shim.go b/usb/hidapi_shim.go index d6247072..2cd3437e 100644 --- a/usb/hidapi_shim.go +++ b/usb/hidapi_shim.go @@ -41,7 +41,7 @@ func (d *HID) Write(buf []byte) (int, error) { panic("not implemented for linux and freebsd") } -func (d *HID) Read(buf []byte) (int, error) { +func (d *HID) Read([]byte, bool) (int, error) { panic("not implemented for linux and freebsd") } diff --git a/usb/libusb.go b/usb/libusb.go index c8bb6386..e8682ff1 100644 --- a/usb/libusb.go +++ b/usb/libusb.go @@ -66,7 +66,8 @@ func InitLibUSB(mw *memorywriter.MemoryWriter, onlyLibusb, allowCancel, detach b err := lowlevel.Init(&usb) if err != nil { return nil, fmt.Errorf(`error when initializing LibUSB. -If you run trezord in an environment without USB (for example, docker or travis), use '-u=false'. For example, './trezord-go -e 21324 -u=false'. +If you run trezord in an environment without USB (for example, docker or travis), +use '-u=false'. For example, './trezord-go -e 21324 -u=false'. Original error: %v`, err) } @@ -160,8 +161,8 @@ func (b *LibUSB) Enumerate() ([]core.USBInfo, error) { } infos = append(infos, core.USBInfo{ Path: path, - VendorID: int(dd.IdVendor), - ProductID: int(dd.IdProduct), + VendorID: int(dd.IDVendor), + ProductID: int(dd.IDProduct), Type: t, Debug: debug, }) @@ -294,8 +295,9 @@ func (b *LibUSB) connect(dev lowlevel.Device, debug bool, reset bool) (*LibUSBDe if err != nil { return nil, err } - b.mw.Log("reset") + b.mw.Log("reset ?") if reset { + b.mw.Log("reset .") err = lowlevel.Reset_Device(d) if err != nil { // don't abort if reset fails @@ -323,12 +325,12 @@ func (b *LibUSB) connect(dev lowlevel.Device, debug bool, reset bool) (*LibUSBDe } func matchType(dd *lowlevel.Device_Descriptor) core.DeviceType { - if dd.IdProduct == core.ProductT1Firmware { + if dd.IDProduct == core.ProductT1Firmware { // this is HID, in platforms where we don't use hidapi (linux, bsd) return core.TypeT1Hid } - if dd.IdProduct == core.ProductT2Bootloader { + if dd.IDProduct == core.ProductT2Bootloader { if int(dd.BcdDevice>>8) == 1 { return core.TypeT1WebusbBoot } @@ -350,8 +352,8 @@ func (b *LibUSB) match(dev lowlevel.Device) (bool, core.DeviceType) { return false, 0 } - vid := dd.IdVendor - pid := dd.IdProduct + vid := dd.IDVendor + pid := dd.IDProduct if !b.matchVidPid(vid, pid) { b.mw.Log("unmatched") return false, 0 @@ -441,16 +443,13 @@ func (d *LibUSBDevice) Close(disconnected bool) error { d.mw.Log("canceling previous transfers") lowlevel.Cancel_Sync_Transfers_On_Device(d.dev) + } - // reading recently disconnected device sometimes causes weird issues - // => if we *know* it is disconnected, don't finish read queue - // - // Finishing read queue is not necessary when we don't allow cancelling - // (since when we don't allow cancelling, we don't allow session stealing) - if !disconnected { - d.mw.Log("finishing read queue") - d.finishReadQueue(d.debug) - } + // reading recently disconnected device sometimes causes weird issues + // => if we *know* it is disconnected, don't finish read queue + if !disconnected { + d.mw.Log("finishing read queue") + d.finishReadQueue(d.debug) } d.mw.Log("releasing interface") @@ -495,24 +494,32 @@ func (d *LibUSBDevice) transferMutexUnlock(debug bool) { } } -func (d *LibUSBDevice) finishReadQueue(debug bool) { +func (d *LibUSBDevice) shortRead(buf []byte, debug bool) (int, error) { d.mw.Log("wait for transfermutex lock") usbEpIn := normalIface.epIn if debug { usbEpIn = debugIface.epIn } d.transferMutexLock(debug) + // these transfers have timeouts => should not interfer with + // cancel_sync_transfers_on_device + d.mw.Log("transfer") + i, err := lowlevel.Interrupt_Transfer(d.dev, usbEpIn, buf, 50) + d.transferMutexUnlock(debug) + d.mw.Log("done") + if err != nil && isErrorTimeout(err) { + return 0, core.ErrTimeout + } + return len(i), err +} + +func (d *LibUSBDevice) finishReadQueue(debug bool) { var err error var buf [64]byte for err == nil { - // these transfers have timeouts => should not interfer with - // cancel_sync_transfers_on_device - d.mw.Log("transfer") - _, err = lowlevel.Interrupt_Transfer(d.dev, usbEpIn, buf[:], 50) + _, err = d.shortRead(buf[:], debug) } - d.transferMutexUnlock(debug) - d.mw.Log("done") } func (d *LibUSBDevice) readWrite(buf []byte, endpoint uint8) (int, error) { @@ -555,6 +562,10 @@ func (d *LibUSBDevice) readWrite(buf []byte, endpoint uint8) (int, error) { } } +func isErrorTimeout(err error) bool { + return (err.Error() == lowlevel.Error_Name(int(lowlevel.ERROR_TIMEOUT))) +} + func isErrorDisconnect(err error) bool { // according to libusb docs, disconnecting device should cause only // LIBUSB_ERROR_NO_DEVICE error, but in real life, it causes also @@ -578,11 +589,14 @@ func (d *LibUSBDevice) Write(buf []byte) (int, error) { return d.readWrite(buf, usbEpOut) } -func (d *LibUSBDevice) Read(buf []byte) (int, error) { +func (d *LibUSBDevice) Read(buf []byte, stopShortTimeout bool) (int, error) { d.mw.Log("read start") usbEpIn := normalIface.epIn if d.debug { usbEpIn = debugIface.epIn } + if stopShortTimeout { + return d.shortRead(buf, d.debug) + } return d.readWrite(buf, usbEpIn) } diff --git a/usb/lowlevel/hidapi/hid.go b/usb/lowlevel/hidapi/hid.go index 3529dd7a..9fbba96d 100644 --- a/usb/lowlevel/hidapi/hid.go +++ b/usb/lowlevel/hidapi/hid.go @@ -73,7 +73,7 @@ type HidDeviceInfo struct { // > "subsequent calls will cause the hid manager to release previously enumerated devices" var enumerateLock sync.Mutex -func init() { +func Init() { // Initialize the HIDAPI library C.hid_init() } diff --git a/usb/lowlevel/hidapi/log.go b/usb/lowlevel/hidapi/log.go index c6c8b55f..6b99b62a 100644 --- a/usb/lowlevel/hidapi/log.go +++ b/usb/lowlevel/hidapi/log.go @@ -1,6 +1,9 @@ +// +build darwin,!ios,cgo windows,cgo + package hidapi import ( + "fmt" "io" ) @@ -15,6 +18,10 @@ func SetLogWriter(l io.Writer) { //export goHidLog func goHidLog(s *C.char) { if writer != nil { - writer.Write([]byte(C.GoString(s))) + _, err := writer.Write([]byte(C.GoString(s))) + if err != nil { + // whatever, just log it out + fmt.Println(err) + } } } diff --git a/usb/lowlevel/hidapi/wchar.go b/usb/lowlevel/hidapi/wchar.go index 2278c6b9..69cafb4f 100644 --- a/usb/lowlevel/hidapi/wchar.go +++ b/usb/lowlevel/hidapi/wchar.go @@ -6,8 +6,7 @@ // The vendored file is licensed under the 3-clause BSD license, according to: // https://github.com/orofarne/gowchar/blob/master/LICENSE -// +build !ios -// +build linux freebsd darwin windows +// +build darwin,!ios,cgo windows,cgo package hidapi @@ -34,18 +33,7 @@ import ( "unicode/utf8" ) -var sizeofWcharT C.size_t = C.size_t(C.SIZEOF_WCHAR_T) - -func stringToWcharT(s string) (*C.wchar_t, C.size_t) { - switch sizeofWcharT { - case 2: - return stringToWchar2(s) // Windows - case 4: - return stringToWchar4(s) // Unix - default: - panic(fmt.Sprintf("Invalid sizeof(wchar_t) = %v", sizeofWcharT)) - } -} +var sizeofWcharT C.size_t = C.SIZEOF_WCHAR_T func wcharTToString(s *C.wchar_t) (string, error) { switch sizeofWcharT { @@ -58,66 +46,6 @@ func wcharTToString(s *C.wchar_t) (string, error) { } } -func wcharTNToString(s *C.wchar_t, size C.size_t) (string, error) { - switch sizeofWcharT { - case 2: - return wchar2NToString(s, size) // Windows - case 4: - return wchar4NToString(s, size) // Unix - default: - panic(fmt.Sprintf("Invalid sizeof(wchar_t) = %v", sizeofWcharT)) - } -} - -// Windows -func stringToWchar2(s string) (*C.wchar_t, C.size_t) { - var slen int - s1 := s - for len(s1) > 0 { - r, size := utf8.DecodeRuneInString(s1) - if er, _ := utf16.EncodeRune(r); er == '\uFFFD' { - slen += 1 - } else { - slen += 2 - } - s1 = s1[size:] - } - slen++ // \0 - res := C.malloc(C.size_t(slen) * sizeofWcharT) - var i int - for len(s) > 0 { - r, size := utf8.DecodeRuneInString(s) - if r1, r2 := utf16.EncodeRune(r); r1 != '\uFFFD' { - C.gowchar_set((*C.wchar_t)(res), C.int(i), C.wchar_t(r1)) - i++ - C.gowchar_set((*C.wchar_t)(res), C.int(i), C.wchar_t(r2)) - i++ - } else { - C.gowchar_set((*C.wchar_t)(res), C.int(i), C.wchar_t(r)) - i++ - } - s = s[size:] - } - C.gowchar_set((*C.wchar_t)(res), C.int(slen-1), C.wchar_t(0)) // \0 - return (*C.wchar_t)(res), C.size_t(slen) -} - -// Unix -func stringToWchar4(s string) (*C.wchar_t, C.size_t) { - slen := utf8.RuneCountInString(s) - slen++ // \0 - res := C.malloc(C.size_t(slen) * sizeofWcharT) - var i int - for len(s) > 0 { - r, size := utf8.DecodeRuneInString(s) - C.gowchar_set((*C.wchar_t)(res), C.int(i), C.wchar_t(r)) - s = s[size:] - i++ - } - C.gowchar_set((*C.wchar_t)(res), C.int(slen-1), C.wchar_t(0)) // \0 - return (*C.wchar_t)(res), C.size_t(slen) -} - // Windows func wchar2ToString(s *C.wchar_t) (string, error) { var i int @@ -131,7 +59,7 @@ func wchar2ToString(s *C.wchar_t) (string, error) { i++ if !utf16.IsSurrogate(r) { if !utf8.ValidRune(r) { - err := fmt.Errorf("Invalid rune at position %v", i) + err := fmt.Errorf("invalid rune at position %v", i) return "", err } res += string(r) @@ -140,7 +68,7 @@ func wchar2ToString(s *C.wchar_t) (string, error) { r2 := rune(ch2) r12 := utf16.DecodeRune(r, r2) if r12 == '\uFFFD' { - err := fmt.Errorf("Invalid surrogate pair at position %v", i-1) + err := fmt.Errorf("invalid surrogate pair at position %v", i-1) return "", err } res += string(r12) @@ -161,63 +89,7 @@ func wchar4ToString(s *C.wchar_t) (string, error) { } r := rune(ch) if !utf8.ValidRune(r) { - err := fmt.Errorf("Invalid rune at position %v", i) - return "", err - } - res += string(r) - i++ - } - return res, nil -} - -// Windows -func wchar2NToString(s *C.wchar_t, size C.size_t) (string, error) { - var i int - var res string - N := int(size) - for i < N { - ch := C.gowchar_get(s, C.int(i)) - if ch == 0 { - break - } - r := rune(ch) - i++ - if !utf16.IsSurrogate(r) { - if !utf8.ValidRune(r) { - err := fmt.Errorf("Invalid rune at position %v", i) - return "", err - } - - res += string(r) - } else { - if i >= N { - err := fmt.Errorf("Invalid surrogate pair at position %v", i-1) - return "", err - } - ch2 := C.gowchar_get(s, C.int(i)) - r2 := rune(ch2) - r12 := utf16.DecodeRune(r, r2) - if r12 == '\uFFFD' { - err := fmt.Errorf("Invalid surrogate pair at position %v", i-1) - return "", err - } - res += string(r12) - i++ - } - } - return res, nil -} - -// Unix -func wchar4NToString(s *C.wchar_t, size C.size_t) (string, error) { - var i int - var res string - N := int(size) - for i < N { - ch := C.gowchar_get(s, C.int(i)) - r := rune(ch) - if !utf8.ValidRune(r) { - err := fmt.Errorf("Invalid rune at position %v", i) + err := fmt.Errorf("invalid rune at position %v", i) return "", err } res += string(r) diff --git a/usb/lowlevel/libusb/libusb.go b/usb/lowlevel/libusb/libusb.go index 6a159ecb..1b199079 100644 --- a/usb/lowlevel/libusb/libusb.go +++ b/usb/lowlevel/libusb/libusb.go @@ -114,9 +114,8 @@ import ( func bcd2str(x uint16) string { if (x>>12)&15 != 0 { return fmt.Sprintf("%d%d.%d%d", (x>>12)&15, (x>>8)&15, (x>>4)&15, (x>>0)&15) - } else { - return fmt.Sprintf("%d.%d%d", (x>>8)&15, (x>>4)&15, (x>>0)&15) } + return fmt.Sprintf("%d.%d%d", (x>>8)&15, (x>>4)&15, (x>>0)&15) } func indent(s string) string { @@ -591,7 +590,7 @@ func (x *Config_Descriptor) String() string { s = append(s, fmt.Sprintf("MaxPower %d", x.MaxPower)) for i, v := range x.Interface { s = append(s, fmt.Sprintf("Interface %d:", i)) - s = append(s, indent(fmt.Sprintf(Interface_str(v)))) + s = append(s, indent(Interface_str(v))) } s = append(s, fmt.Sprintf("extra %s", Extra_str(x.Extra))) return strings.Join(s, "\n") @@ -783,8 +782,8 @@ type Device_Descriptor struct { BDeviceSubClass uint8 BDeviceProtocol uint8 BMaxPacketSize0 uint8 - IdVendor uint16 - IdProduct uint16 + IDVendor uint16 + IDProduct uint16 BcdDevice uint16 IManufacturer uint8 IProduct uint8 @@ -802,8 +801,8 @@ func (x *C.struct_libusb_device_descriptor) c2go() *Device_Descriptor { BDeviceSubClass: uint8(x.bDeviceSubClass), BDeviceProtocol: uint8(x.bDeviceProtocol), BMaxPacketSize0: uint8(x.bMaxPacketSize0), - IdVendor: uint16(x.idVendor), - IdProduct: uint16(x.idProduct), + IDVendor: uint16(x.idVendor), + IDProduct: uint16(x.idProduct), BcdDevice: uint16(x.bcdDevice), IManufacturer: uint8(x.iManufacturer), IProduct: uint8(x.iProduct), @@ -822,8 +821,8 @@ func (x *Device_Descriptor) String() string { s = append(s, fmt.Sprintf("bDeviceSubClass %d", x.BDeviceSubClass)) s = append(s, fmt.Sprintf("bDeviceProtocol %d", x.BDeviceProtocol)) s = append(s, fmt.Sprintf("bMaxPacketSize0 %d", x.BMaxPacketSize0)) - s = append(s, fmt.Sprintf("idVendor 0x%04x", x.IdVendor)) - s = append(s, fmt.Sprintf("idProduct 0x%04x", x.IdProduct)) + s = append(s, fmt.Sprintf("idVendor 0x%04x", x.IDVendor)) + s = append(s, fmt.Sprintf("idProduct 0x%04x", x.IDProduct)) s = append(s, fmt.Sprintf("bcdDevice %s", bcd2str(x.BcdDevice))) s = append(s, fmt.Sprintf("iManufacturer %d", x.IManufacturer)) s = append(s, fmt.Sprintf("iProduct %d", x.IProduct)) @@ -951,7 +950,7 @@ func Exit(ctx Context) { func Get_Device_List(ctx Context) ([]Device, error) { var hdl **C.struct_libusb_device - rc := int(C.libusb_get_device_list(ctx, (***C.struct_libusb_device)(&hdl))) + rc := int(C.libusb_get_device_list(ctx, &hdl)) if rc < 0 { return nil, &libusb_error{rc} } diff --git a/usb/lowlevel/libusb/log.go b/usb/lowlevel/libusb/log.go index f0a0900e..9cebc0d0 100644 --- a/usb/lowlevel/libusb/log.go +++ b/usb/lowlevel/libusb/log.go @@ -1,6 +1,7 @@ package libusb import ( + "fmt" "io" ) @@ -15,6 +16,10 @@ func SetLogWriter(l io.Writer) { //export goLibusbLog func goLibusbLog(s *C.char) { if writer != nil { - writer.Write([]byte(C.GoString(s))) + _, err := writer.Write([]byte(C.GoString(s))) + if err != nil { + // whatever, just log it out + fmt.Println(err) + } } } diff --git a/usb/udp.go b/usb/udp.go index 5339b9cc..ce21979b 100644 --- a/usb/udp.go +++ b/usb/udp.go @@ -196,7 +196,7 @@ func (d *UDPDevice) Close(disconnected bool) error { return nil } -func (d *UDPDevice) readWrite(buf []byte, read bool) (int, error) { +func (d *UDPDevice) readWrite(buf []byte, read bool, stopShortTimeout bool) (int, error) { lowlevel := d.lowlevel for { closed := (atomic.LoadInt32(&d.closed)) == 1 @@ -219,15 +219,18 @@ func (d *UDPDevice) readWrite(buf []byte, read bool) (int, error) { copy(buf, response) return len(response), nil case <-time.After(emulatorPingTimeout): + if stopShortTimeout { + return 0, core.ErrTimeout + } // timeout, continue for cycle } } } func (d *UDPDevice) Write(buf []byte) (int, error) { - return d.readWrite(buf, false) + return d.readWrite(buf, false, false) } -func (d *UDPDevice) Read(buf []byte) (int, error) { - return d.readWrite(buf, true) +func (d *UDPDevice) Read(buf []byte, stopShortTimeout bool) (int, error) { + return d.readWrite(buf, true, stopShortTimeout) }