github-vet / github-vet/rangeloop-pointer-findings

sqweek/sqribe: ww_ev.go; 70 LoC

Open
#17,296 0 comments 0 reactions 0 assignees View on GitHub
fresh medium
Dominant language
No language data
Stars
0
Forks
0
PR merge metrics
PR metrics pending

Description

Found a possible issue in [sqweek/sqribe](https://www.github.com/sqweek/sqribe) at [ww_ev.go](https://github.com/sqweek/sqribe/blob/920a6cdadac38cbeafd130cb059a07c6308cd59c/ww_ev.go#L99-L168)

Below is the message reported by the analyzer for this snippet of code. Beware that the analyzer only reports the first issue it finds, so please do not limit your consideration to the contents of the below message.

> range-loop variable staff used in defer or goroutine at line 107

[Click here to see the code in its original context.](https://github.com/sqweek/sqribe/blob/920a6cdadac38cbeafd130cb059a07c6308cd59c/ww_ev.go#L99-L168)

Click here to show the 70 line(s) of Go which triggered the analyzer.

```go
for staff, slayout := range staves {
if e.Where.In(slayout.mix.instC) {
G.instMenu.SetDefault(Mixer.For(staff).Voice)
reply := G.instMenu.Popup(slayout.mix.r, ww.refresh, e.Where)
go func() {
item := <-reply
id, ok := item.(int)
if item != nil && ok {
Mixer.For(staff).Voice = id
}
ww.changed(MIXER, ww)
}()
return G.instMenu.Drag
} else if e.Where.In(slayout.mix.volS) {
return func(pos image.Point, finished bool, moved bool)bool {
slider := slayout.mix.volS
if (moved || finished) && pos.In(slider) {
α := float64(pos.Y - slider.Min.Y) / float64(slider.Dy())
vel := 127 - int(127.0 * α + 0.5)
Mixer.For(staff).Velocity = vel
ww.changed(MIXER, ww)
return true
}
return false
}
} else if e.Where.In(slayout.mix.minmaxB) {
overlay := NoOverlay
return func(pos image.Point, finished bool, moved bool)bool {
if (finished && !moved) { /* regular click */
switch e.Which {
case wde.LeftButton:
toggle(&slayout.mix.Minimised)
ww.changed(LAYOUT | SCALE, &slayout.mix.Minimised)
case wde.RightButton:
slayout.mix.Minimised = false
for staff2, slayout2 := range staves {
if staff2 != staff {
slayout2.mix.Minimised = true
}
}
ww.changed(LAYOUT | SCALE, &slayout.mix.Minimised)
}
}
if overlay == NoOverlay {
overlay = G.overlay.Make()
}
ok, border, fill := false, color.NRGBA{0xff, 0x00, 0x00, 0xcc}, color.NRGBA{0x88, 0x55, 0x55, 0xcc}
if pos.In(ww.rect.mixer) {
if !pos.In(slayout.mix.r) {
ok = true
border, fill = color.NRGBA{0x00, 0xff, 0x00, 0xcc}, color.NRGBA{0x55, 0x88, 0x55, 0xcc}
if finished {
for staff2, slayout2 := range ww.rect.staves() {
if pos.In(slayout2.mix.r) {
ww.MoveStaffTo(staff, staff2)
break
}
}
}
}
}
if finished {
overlay.Close(!ok)
} else {
overlay.Update(boxDrawable(pos, slayout.mix.minmaxB.Dx()/2, slayout.mix.minmaxB.Dy()/2, border, fill))
}
return ok
}
}
}

```

Leave a reaction on this issue to contribute to the project by classifying this instance as a **Bug** :-1:, **Mitigated** :+1:, or **Desirable Behavior** :rocket:
See the descriptions of the classifications [here](https://github.com/github-vet/rangeclosure-findings#how-can-i-help) for more information.

commit ID: 920a6cdadac38cbeafd130cb059a07c6308cd59c

Contributor guide

No contributing guide indexed for this repository

Research direction

Open ww_ev.go at lines 99-168 in commit 920a6cdadac38cbeafd130cb059a07c6308cd59c and trace the range-loop variable staff through the goroutine and returned closures. Confirm whether each callback uses the intended staff value, then verify that the analyzer no longer reports the range-loop capture and that the affected mixer interactions still target the correct staff.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
desktop
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.