Skip to content

Commit 191ae9e

Browse files
committed
Fix possible crash when deleting remote usecases
When deleting a remote devices usecases, deletion for the internal data structure should be done in reverse order as indices are used. Otherwise deleting multiple elements could result in a crash.
1 parent d5a3838 commit 191ae9e

File tree

3 files changed

+41
-9
lines changed

3 files changed

+41
-9
lines changed

usecases/usecase/testhelper_test.go

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,18 @@ func setupDevices(
151151
f.AddFunctionType(model.FunctionTypeDeviceClassificationUserData, true, true)
152152
localEntity.AddFeature(f)
153153

154+
remoteDeviceName := "remote"
155+
156+
remoteDevice, entities := addRemoteDevice(remoteDeviceName, remoteSki, localDevice, t)
157+
158+
localDevice.AddRemoteDeviceForSki(remoteSki, remoteDevice)
159+
160+
return localEntity, remoteDevice, entities
161+
}
162+
163+
func addRemoteDevice(remoteDeviceName, remoteDeviceSki string, localDevice spineapi.DeviceLocalInterface, t *testing.T) (
164+
spineapi.DeviceRemoteInterface,
165+
[]spineapi.EntityRemoteInterface) {
154166
writeHandler := shipmocks.NewShipConnectionDataWriterInterface(t)
155167
writeHandler.EXPECT().WriteShipMessageWithPayload(mock.Anything).Return().Maybe()
156168
sender := spine.NewSender(writeHandler)
@@ -199,8 +211,6 @@ func setupDevices(
199211
},
200212
}
201213

202-
remoteDeviceName := "remote"
203-
204214
var featureInformations []model.NodeManagementDetailedDiscoveryFeatureInformationType
205215
for index, feature := range remoteFeatures {
206216
supportedFcts := []model.FunctionPropertyType{}
@@ -270,7 +280,7 @@ func setupDevices(
270280
entity.UpdateDeviceAddress(*remoteDevice.Address())
271281
}
272282

273-
localDevice.AddRemoteDeviceForSki(remoteSki, remoteDevice)
283+
localDevice.AddRemoteDeviceForSki(remoteDeviceSki, remoteDevice)
274284

275-
return localEntity, remoteDevice, entities
285+
return remoteDevice, entities
276286
}

usecases/usecase/usecase.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -223,8 +223,8 @@ func (u *UseCaseBase) updateRemoteEntityScenarios(
223223
func (u *UseCaseBase) removeDeviceFromAvailableEntityScenarios(device spineapi.DeviceRemoteInterface) {
224224
indicies := u.entityScenarioIndicesOfDevice(device)
225225

226-
for _, i := range indicies {
227-
u.removeEntityIndexFromAvailableEntityScenarios(i)
226+
for i := len(indicies) - 1; i >= 0; i-- {
227+
u.removeEntityIndexFromAvailableEntityScenarios(indicies[i])
228228
}
229229

230230
if u.EventCB != nil && len(indicies) > 0 {

usecases/usecase/usecase_test.go

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,15 @@ func (s *UseCaseSuite) Test() {
4444
}
4545

4646
func (s *UseCaseSuite) Test_RemoveDeviceScenarios() {
47+
remoteDevice2Ski := "remoteDevice2Ski"
48+
localDevice := s.localEntity.Device()
49+
remoteDevice2, remoteEntities2 := addRemoteDevice("remoteDevice2", remoteDevice2Ski, localDevice, s.T())
50+
51+
localDevice.AddRemoteDeviceForSki(remoteDevice2Ski, remoteDevice2)
52+
53+
entity1 := remoteEntities2[0]
54+
entity2 := remoteEntities2[1]
55+
4756
result := s.uc.RemoteEntitiesScenarios()
4857
assert.Equal(s.T(), 0, len(result))
4958

@@ -53,20 +62,33 @@ func (s *UseCaseSuite) Test_RemoveDeviceScenarios() {
5362
ok := s.uc.IsScenarioAvailableAtEntity(s.monitoredEntity, 1)
5463
assert.False(s.T(), ok)
5564

56-
s.uc.updateRemoteEntityScenarios(s.monitoredEntity, []model.UseCaseScenarioSupportType{1, 2, 3})
65+
s.uc.updateRemoteEntityScenarios(entity1, []model.UseCaseScenarioSupportType{1, 2, 3, 4})
5766

5867
result = s.uc.RemoteEntitiesScenarios()
5968
assert.Equal(s.T(), 1, len(result))
6069

70+
s.uc.updateRemoteEntityScenarios(s.monitoredEntity, []model.UseCaseScenarioSupportType{1, 2, 3})
71+
72+
result = s.uc.RemoteEntitiesScenarios()
73+
assert.Equal(s.T(), 2, len(result))
74+
6175
scenarios = s.uc.AvailableScenariosForEntity(s.monitoredEntity)
6276
assert.Equal(s.T(), 3, len(scenarios))
6377

64-
s.uc.removeDeviceFromAvailableEntityScenarios(s.monitoredEntity.Device())
78+
s.uc.updateRemoteEntityScenarios(entity2, []model.UseCaseScenarioSupportType{1, 2})
6579

6680
result = s.uc.RemoteEntitiesScenarios()
67-
assert.Equal(s.T(), 0, len(result))
81+
assert.Equal(s.T(), 3, len(result))
82+
83+
s.uc.removeDeviceFromAvailableEntityScenarios(entity2.Device())
84+
85+
result = s.uc.RemoteEntitiesScenarios()
86+
assert.Equal(s.T(), 1, len(result))
6887

6988
scenarios = s.uc.AvailableScenariosForEntity(s.monitoredEntity)
89+
assert.Equal(s.T(), 3, len(scenarios))
90+
91+
scenarios = s.uc.AvailableScenariosForEntity(entity2)
7092
assert.Equal(s.T(), 0, len(scenarios))
7193
}
7294

0 commit comments

Comments
 (0)