Skip to content

Commit

Permalink
Do not assume interface numbers follow the slice indices. (#134)
Browse files Browse the repository at this point in the history
Do not assume interface numbers follow the slice indices.

This is a continuation of
9ad5483
which tried to solve the problem of non-contiguous interface indices;
this commit modifies another code path that had the same assumption.
  • Loading branch information
zagrodzki authored Sep 13, 2024
1 parent 606016a commit e840be9
Show file tree
Hide file tree
Showing 4 changed files with 91 additions and 22 deletions.
28 changes: 12 additions & 16 deletions config.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,25 +49,17 @@ func (c ConfigDesc) String() string {
return fmt.Sprintf("Configuration %d", c.Number)
}

func (c ConfigDesc) intfDesc(num, alt int) (*InterfaceSetting, error) {
func (c ConfigDesc) intfDesc(num int) (*InterfaceDesc, error) {
// In an ideal world, interfaces in the descriptor would be numbered
// contiguously starting from 0, as required by the specification. In the
// real world however the specification is sometimes ignored:
// https://github.com/google/gousb/issues/65
ifs := make([]int, len(c.Interfaces))
for i := range c.Interfaces {
ifs[i] = c.Interfaces[i].Number
if ifs[i] != num {
continue
for i, desc := range c.Interfaces {
if desc.Number == num {
return &desc, nil
}
alts := make([]int, len(c.Interfaces[i].AltSettings))
for a := range c.Interfaces[i].AltSettings {
alts[a] = c.Interfaces[i].AltSettings[a].Alternate
if alts[a] == alt {
return &c.Interfaces[i].AltSettings[a], nil
}
}
return nil, fmt.Errorf("alternate setting %d not found for %s, available alt settings: %v", alt, c.Interfaces[i], alts)
ifs[i] = desc.Number
}
return nil, fmt.Errorf("interface %d not found, available interface numbers: %v", num, ifs)
}
Expand Down Expand Up @@ -120,9 +112,13 @@ func (c *Config) Interface(num, alt int) (*Interface, error) {
return nil, fmt.Errorf("Interface(%d, %d) called on %s after Close", num, alt, c)
}

altInfo, err := c.Desc.intfDesc(num, alt)
intf, err := c.Desc.intfDesc(num)
if err != nil {
return nil, fmt.Errorf("descriptor of interface %d in %s: %v", num, c, err)
}
altInfo, err := intf.altSetting(alt)
if err != nil {
return nil, fmt.Errorf("descriptor of interface (%d, %d) in %s: %v", num, alt, c, err)
return nil, fmt.Errorf("descriptor of alternate setting %d of interface %d in %s: %v", alt, num, c, err)
}

c.mu.Lock()
Expand All @@ -137,7 +133,7 @@ func (c *Config) Interface(num, alt int) (*Interface, error) {
}

// Select an alternate setting if needed (device has multiple alternate settings).
if len(c.Desc.Interfaces[num].AltSettings) > 1 {
if len(intf.AltSettings) > 1 {
if err := c.dev.ctx.libusb.setAlt(c.dev.handle, uint8(num), uint8(alt)); err != nil {
c.dev.ctx.libusb.release(c.dev.handle, uint8(num))
return nil, fmt.Errorf("failed to set alternate config %d on interface %d of %s: %v", alt, num, c, err)
Expand Down
8 changes: 6 additions & 2 deletions device.go
Original file line number Diff line number Diff line change
Expand Up @@ -276,9 +276,13 @@ func (d *Device) InterfaceDescription(cfgNum, intfNum, altNum int) (string, erro
if err != nil {
return "", fmt.Errorf("%s: %v", d, err)
}
alt, err := cfg.intfDesc(intfNum, altNum)
intf, err := cfg.intfDesc(intfNum)
if err != nil {
return "", fmt.Errorf("%s, configuration %d: %v", d, cfgNum, err)
return "", fmt.Errorf("%s, configuration %d interface %d: %v", d, cfgNum, intfNum, err)
}
alt, err := intf.altSetting(altNum)
if err != nil {
return "", fmt.Errorf("%s, configuration %d interface %d alternate setting %d: %v", d, cfgNum, intfNum, altNum, err)
}
return d.GetStringDescriptor(alt.iInterface)
}
Expand Down
66 changes: 62 additions & 4 deletions device_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,26 +117,26 @@ func TestClaimAndRelease(t *testing.T) {
t.Errorf("%s.InEndpoint(%d): got %+v, want %+v", intf, ep1Addr, got, want)
}

if _, err := cfg.Interface(1, 0); err == nil {
if _, err := cfg.Interface(if1Num, 0); err == nil {
t.Fatalf("%s.Interface(1, 0): got nil, want non nil, because Interface 1 is already claimed.", cfg)
}

// intf2 is interface #0, not claimed yet.
intf2, err := cfg.Interface(0, 0)
intf2, err := cfg.Interface(if2Num, alt2Num)
if err != nil {
t.Fatalf("%s.Interface(0, 0): got %v, want nil", cfg, err)
}

if err := cfg.Close(); err == nil {
t.Fatalf("%s.Close(): got nil, want non nil, because the Interface was not released.", cfg)
t.Fatalf("%s.Close(): got nil, want non nil, because the Interfaces #1/2 was not released.", cfg)
}
if err := dev.Close(); err == nil {
t.Fatalf("%s.Close(): got nil, want non nil, because the Config was not released.", cfg)
}

intf.Close()
if err := cfg.Close(); err == nil {
t.Fatalf("%s.Close(): got nil, want non nil, because the Interface was not released.", cfg)
t.Fatalf("%s.Close(): got nil, want non nil, because the Interface #2 was not released.", cfg)
}
if err := dev.Close(); err == nil {
t.Fatalf("%s.Close(): got nil, want non nil, because the Config was not released.", dev)
Expand Down Expand Up @@ -244,3 +244,61 @@ func TestAutoDetachFailure(t *testing.T) {
t.Fatalf("%s.Config(1) got nil, but want no nil because interface fails to detach", dev)
}
}

func TestInterface(t *testing.T) {
t.Parallel()
c := newContextWithImpl(newFakeLibusb())
defer func() {
if err := c.Close(); err != nil {
t.Errorf("Context.Close: %v", err)
}
}()

for _, tc := range []struct {
name string
vid, pid ID
cfgNum, ifNum, altNum int
}{{
name: "simple",
vid: 0x8888,
pid: 0x0002,
cfgNum: 1,
ifNum: 0,
altNum: 0,
}, {
name: "alt_setting",
vid: 0x8888,
pid: 0x0002,
cfgNum: 1,
ifNum: 1,
altNum: 1,
}, {
name: "noncontiguous_interfaces",
vid: 0x8888,
pid: 0x0002,
cfgNum: 1,
ifNum: 3,
altNum: 2,
}} {
t.Run(tc.name, func(t *testing.T) {
dev, err := c.OpenDeviceWithVIDPID(0x8888, 0x0002)
if err != nil {
t.Fatalf("OpenDeviceWithVIDPID(0x8888, 0x0002): %v", err)
}
if dev == nil {
t.Fatal("OpenDeviceWithVIDPID(0x8888, 0x0002): got nil device, need non-nil")
}
defer dev.Close()
cfg, err := dev.Config(tc.cfgNum)
if err != nil {
t.Fatalf("%s.Config(%d): %v", dev, tc.cfgNum, err)
}
defer cfg.Close()
intf, err := cfg.Interface(tc.ifNum, tc.altNum)
if err != nil {
t.Fatalf("%s.Interface(%d, %d): %v", cfg, tc.ifNum, tc.altNum, err)
}
intf.Close()
})
}
}
11 changes: 11 additions & 0 deletions interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,17 @@ type InterfaceDesc struct {
AltSettings []InterfaceSetting
}

func (i *InterfaceDesc) altSetting(alt int) (*InterfaceSetting, error) {
alts := make([]int, len(i.AltSettings))
for a, s := range i.AltSettings {
if s.Alternate == alt {
return &s, nil
}
alts[a] = s.Alternate
}
return nil, fmt.Errorf("alternate setting %d not found for %s, available alt settings: %v", alt, i, alts)
}

// String returns a human-readable description of the interface descriptor and
// its alternate settings.
func (i InterfaceDesc) String() string {
Expand Down

0 comments on commit e840be9

Please sign in to comment.