elki-project / elki

ELKI Data Mining Toolkit
https://elki-project.github.io/
GNU Affero General Public License v3.0
785 stars 323 forks source link

Suspicious code fragments found by PVS-Studio #107

Closed VasilievSerg closed 1 year ago

VasilievSerg commented 1 year ago

I've created POC for a few suspicious code fragments found by the PVS-Studio static code analyzer. They're below.

Issue 1 Two parameters of the DataStoreEvent constructor are not used. As a result, the object state after initialization is incorrect.

Code:

public DataStoreEvent(DBIDs inserts, DBIDs removals, DBIDs updates) {
    super();
    this.inserts = inserts;
    this.removals = inserts; // <=
    this.updates = inserts;  // <=
}

Link to the sources.

POC ``` var inserts = DBIDUtil.newHashSet(32); var removals = DBIDUtil.newHashSet(64); var updates = DBIDUtil.newHashSet(128); var dataStoreEvent = new DataStoreEvent(inserts, removals, updates); // True: inserts.equals(dataStoreEvent.getInserts()); // False: removals.equals(dataStoreEvent.getRemovals()); // False: updates.equals(dataStoreEvent.getUpdates()); ```

Issue 2 The setInitialMeans method recurses infinitely.

Code:

public void setInitialMeans(List<double[]> initialMeans) {
  this.setInitialMeans(initialMeans);
}

Link to the sources.

POC ``` var predef = new Predefined(new double[3][3]); var arrList = new ArrayList(); predef.setInitialMeans(arrList); // stack overflow ```

P.S. Here is an old article describing more suspicions code fragments. I'm not sure that all described warnings are still actual. Anyway, perhaps the article may be useful to you.

kno10 commented 1 year ago

Thank you. The DataStoreEvent will probably be removed (as this entire functionality currently does not have any users that I know of), the setInitialMeans is dead code, so I fixed this by removing the function.