kdenlive: 4 new CWE-407 defects (0005-0008); deeper scan beyond thumbnail/timeline
kdenlive-0005 PreviewManager m_renderedChunks/m_dirtyChunks QVariantList .contains() O(N^2) MEDIUM 219x kdenlive-0006 TimelineController canceled guides std::find O(G*C) MEDIUM 160x kdenlive-0007 AssetParameterModel m_rows.indexOf in param loops O(P*R) MEDIUM 25x kdenlive-0008 UrlListParamWidget addItemsInSameFolder std::find on QMap values O(E*M) MEDIUM 200x 8/8 unit tests PASS
This commit is contained in:
parent
6e6609dbdb
commit
70741a6157
6 changed files with 377 additions and 0 deletions
|
|
@ -0,0 +1,56 @@
|
|||
# UNDF: (leave blank)
|
||||
# Defect: kdenlive-0005
|
||||
# Component: src/timeline2/view/previewmanager.h + previewmanager.cpp
|
||||
# Pattern: CWE-407 — m_renderedChunks/m_dirtyChunks QVariantList with .contains() in loops
|
||||
# Severity: MEDIUM — O(D*M) chunk dedup in reloadChunks, O(N*(R+D)) in invalidatePreview/addPreviewRange/gotChunks
|
||||
# Fix: Maintain shadow QSet<int> for O(1) membership tests on m_renderedChunks and m_dirtyChunks
|
||||
--- a/src/timeline2/view/previewmanager.h
|
||||
+++ b/src/timeline2/view/previewmanager.h
|
||||
@@ -146,6 +146,8 @@
|
||||
QVariantList m_renderedChunks;
|
||||
QVariantList m_dirtyChunks;
|
||||
QList<int> m_dirtyChunksToRemove;
|
||||
+ QSet<int> m_renderedChunksSet; // shadow set for O(1) .contains()
|
||||
+ QSet<int> m_dirtyChunksSet; // shadow set for O(1) .contains()
|
||||
|
||||
--- a/src/timeline2/view/previewmanager.cpp
|
||||
+++ b/src/timeline2/view/previewmanager.cpp
|
||||
// Everywhere m_dirtyChunks or m_renderedChunks is modified, also update the shadow set:
|
||||
//
|
||||
// INSERT:
|
||||
// m_dirtyChunks << i;
|
||||
-// → if (!m_dirtyChunksSet.contains(i)) { m_dirtyChunks << i; m_dirtyChunksSet.insert(i); }
|
||||
+// → if (!m_dirtyChunksSet.contains(i)) { m_dirtyChunks << i; m_dirtyChunksSet.insert(i); }
|
||||
//
|
||||
// REMOVE:
|
||||
// m_renderedChunks.removeAll(frame);
|
||||
// → m_renderedChunks.removeAll(frame); m_renderedChunksSet.remove(frame.toInt());
|
||||
//
|
||||
// CONTAINS checks (the CWE-407 sites):
|
||||
//
|
||||
// Line ~188 (reloadChunks):
|
||||
-// if (!m_dirtyChunks.contains(i))
|
||||
+// if (!m_dirtyChunksSet.contains(i.toInt()))
|
||||
//
|
||||
// Line ~472 (invalidatePreview redo lambda):
|
||||
-// if (m_renderedChunks.contains(frame))
|
||||
+// if (m_renderedChunksSet.contains(frame.toInt()))
|
||||
//
|
||||
// Line ~519 (addPreviewRange):
|
||||
-// if (!m_renderedChunks.contains(frame) && !m_dirtyChunks.contains(frame))
|
||||
+// if (!m_renderedChunksSet.contains(frame) && !m_dirtyChunksSet.contains(frame))
|
||||
//
|
||||
// Line ~549 (redo lambda):
|
||||
-// if (m_renderedChunks.contains(frame))
|
||||
+// if (m_renderedChunksSet.contains(frame.toInt()))
|
||||
//
|
||||
// Line ~773 (gotChunks):
|
||||
-// if (m_renderedChunks.contains(i))
|
||||
+// if (m_renderedChunksSet.contains(i))
|
||||
//
|
||||
-// if (!m_dirtyChunks.contains(val))
|
||||
+// if (!m_dirtyChunksSet.contains(i))
|
||||
//
|
||||
// CLEAR sites:
|
||||
// m_renderedChunks.clear(); → also m_renderedChunksSet.clear();
|
||||
// m_dirtyChunks.clear(); → also m_dirtyChunksSet.clear();
|
||||
|
|
@ -0,0 +1,43 @@
|
|||
# UNDF: (leave blank)
|
||||
# Defect: kdenlive-0006
|
||||
# Component: src/timeline2/view/timelinecontroller.cpp
|
||||
# Pattern: CWE-407 — std::find on canceled vector in loop over guides
|
||||
# Severity: MEDIUM — O(G*C) where G=guides, C=canceled/ignored guide positions
|
||||
# Fix: Convert canceled vector to std::unordered_set<int> for O(1) lookup
|
||||
--- a/src/timeline2/view/timelinecontroller.cpp
|
||||
+++ b/src/timeline2/view/timelinecontroller.cpp
|
||||
@@ gotoNextGuide()
|
||||
void TimelineController::gotoNextGuide()
|
||||
{
|
||||
QList<CommentedTime> guides = m_model->getGuideModel()->getAllMarkers();
|
||||
- std::vector<int> canceled = m_model->getFilteredGuideModel()->getIgnoredSnapPoints();
|
||||
+ std::vector<int> canceledVec = m_model->getFilteredGuideModel()->getIgnoredSnapPoints();
|
||||
+ std::unordered_set<int> canceled(canceledVec.begin(), canceledVec.end());
|
||||
int pos = pCore->getMonitorPosition();
|
||||
double fps = pCore->getCurrentFps();
|
||||
int guidePos = 0;
|
||||
for (auto &guide : std::as_const(guides)) {
|
||||
guidePos = guide.time().frames(fps);
|
||||
- if (std::find(canceled.begin(), canceled.end(), guidePos) != canceled.end()) {
|
||||
+ if (canceled.count(guidePos)) {
|
||||
continue;
|
||||
}
|
||||
if (guidePos > pos) {
|
||||
@@ gotoPreviousGuide()
|
||||
void TimelineController::gotoPreviousGuide()
|
||||
{
|
||||
if (pCore->getMonitorPosition() > 0) {
|
||||
QList<CommentedTime> guides = m_model->getGuideModel()->getAllMarkers();
|
||||
- std::vector<int> canceled = m_model->getFilteredGuideModel()->getIgnoredSnapPoints();
|
||||
+ std::vector<int> canceledVec = m_model->getFilteredGuideModel()->getIgnoredSnapPoints();
|
||||
+ std::unordered_set<int> canceled(canceledVec.begin(), canceledVec.end());
|
||||
int pos = pCore->getMonitorPosition();
|
||||
double fps = pCore->getCurrentFps();
|
||||
int lastGuidePos = 0;
|
||||
int guidePos = 0;
|
||||
for (auto &guide : std::as_const(guides)) {
|
||||
guidePos = guide.time().frames(fps);
|
||||
- if (std::find(canceled.begin(), canceled.end(), guidePos) != canceled.end()) {
|
||||
+ if (canceled.count(guidePos)) {
|
||||
continue;
|
||||
}
|
||||
|
|
@ -0,0 +1,55 @@
|
|||
# UNDF: (leave blank)
|
||||
# Defect: kdenlive-0007
|
||||
# Component: src/assets/model/assetparametermodel.hpp + assetparametermodel.cpp
|
||||
# Pattern: CWE-407 — m_rows QVector<QString> with indexOf() called inside loops over m_params/m_fixedParams
|
||||
# Severity: MEDIUM — O(P*R) in getAllParameters, toJson, valueAsJson, setParameters
|
||||
# Fix: Add QHash<QString,int> m_rowIndex shadow map, updated on m_rows changes, for O(1) name-to-row lookup
|
||||
--- a/src/assets/model/assetparametermodel.hpp
|
||||
+++ b/src/assets/model/assetparametermodel.hpp
|
||||
@@ member section
|
||||
QVector<QString> m_rows;
|
||||
+ QHash<QString, int> m_rowIndex; // shadow map: param name → row index for O(1) lookup
|
||||
|
||||
--- a/src/assets/model/assetparametermodel.cpp
|
||||
+++ b/src/assets/model/assetparametermodel.cpp
|
||||
// Wherever m_rows is modified (append, insert, clear), also update m_rowIndex:
|
||||
// m_rows.append(name);
|
||||
// m_rowIndex.insert(name, m_rows.size() - 1);
|
||||
//
|
||||
// Then replace all m_rows.indexOf(name) with m_rowIndex.value(name, -1):
|
||||
//
|
||||
// Line ~611 (setParameter):
|
||||
-// paramIndex = index(m_rows.indexOf(name), 0);
|
||||
+// paramIndex = index(m_rowIndex.value(name, -1), 0);
|
||||
//
|
||||
// Line ~1234 (getAllParameters loop):
|
||||
-// QModelIndex ix = index(m_rows.indexOf(param.first), 0);
|
||||
+// QModelIndex ix = index(m_rowIndex.value(param.first, -1), 0);
|
||||
//
|
||||
// Line ~1320 (toJson fixedParams loop):
|
||||
-// QModelIndex ix = index(m_rows.indexOf(fixed.first), 0);
|
||||
+// QModelIndex ix = index(m_rowIndex.value(fixed.first, -1), 0);
|
||||
//
|
||||
// Line ~1362 (toJson params loop):
|
||||
-// QModelIndex ix = index(m_rows.indexOf(param.first), 0);
|
||||
+// QModelIndex ix = index(m_rowIndex.value(param.first, -1), 0);
|
||||
//
|
||||
// Line ~1492 (valueAsJson fixedParams loop):
|
||||
-// QModelIndex ix = index(m_rows.indexOf(fixed.first), 0);
|
||||
+// QModelIndex ix = index(m_rowIndex.value(fixed.first, -1), 0);
|
||||
//
|
||||
// Line ~1521 (valueAsJson params loop):
|
||||
-// QModelIndex ix = index(m_rows.indexOf(param.first), 0);
|
||||
+// QModelIndex ix = index(m_rowIndex.value(param.first, -1), 0);
|
||||
//
|
||||
// Line ~1797 (setParameters loop):
|
||||
-// QModelIndex ix = index(m_rows.indexOf(param.first), 0);
|
||||
+// QModelIndex ix = index(m_rowIndex.value(param.first, -1), 0);
|
||||
//
|
||||
// Line ~1878 (getParamIndex):
|
||||
-// QModelIndex ix = index(m_rows.indexOf(paramName), 0);
|
||||
+// QModelIndex ix = index(m_rowIndex.value(paramName, -1), 0);
|
||||
//
|
||||
// Line ~1887 (getParamFromName):
|
||||
-// return index(m_rows.indexOf(paramName), 0);
|
||||
+// return index(m_rowIndex.value(paramName, -1), 0);
|
||||
|
|
@ -0,0 +1,36 @@
|
|||
# UNDF: (leave blank)
|
||||
# Defect: kdenlive-0008
|
||||
# Component: src/assets/view/widgets/urllistparamwidget.cpp
|
||||
# Pattern: CWE-407 — std::find iterating QMap values in loop over directory entries
|
||||
# Severity: MEDIUM — O(E*M) where E=directory entries, M=existing map values
|
||||
# Fix: Build QSet<QString> of existing values before the loop for O(1) lookup
|
||||
--- a/src/assets/view/widgets/urllistparamwidget.cpp
|
||||
+++ b/src/assets/view/widgets/urllistparamwidget.cpp
|
||||
@@ addItemsInSameFolder
|
||||
void UrlListParamWidget::addItemsInSameFolder(const QString currentValue, QMap<QString, QString> *listValues)
|
||||
{
|
||||
QDir dir = QFileInfo(currentValue).absoluteDir();
|
||||
if (dir.exists()) {
|
||||
+ // Build a set of existing values for O(1) membership tests
|
||||
+ QSet<QString> existingValues;
|
||||
+ for (auto it = listValues->cbegin(); it != listValues->cend(); ++it) {
|
||||
+ existingValues.insert(it.value());
|
||||
+ }
|
||||
QStringList entries = dir.entryList(m_fileExt, QDir::Files);
|
||||
for (const auto &filename : std::as_const(entries)) {
|
||||
const QString path = dir.absoluteFilePath(filename);
|
||||
- if (std::find((*listValues).cbegin(), (*listValues).cend(), path) == (*listValues).cend()) {
|
||||
+ if (!existingValues.contains(path)) {
|
||||
(*listValues).insert(QFileInfo(filename).baseName(), path);
|
||||
+ existingValues.insert(path);
|
||||
}
|
||||
}
|
||||
// make sure the current value is added. If it is a duplicate we remove it later
|
||||
- if (std::find((*listValues).cbegin(), (*listValues).cend(), currentValue) == (*listValues).cend()) {
|
||||
+ if (!existingValues.contains(currentValue)) {
|
||||
if (QFileInfo::exists(currentValue)) {
|
||||
(*listValues).insert(QFileInfo(currentValue).baseName(), currentValue);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
Binary file not shown.
|
|
@ -6,6 +6,10 @@ import java.util.*;
|
|||
* kdenlive-0002: TimelineModel clipIds vector linear find in mix loop
|
||||
* kdenlive-0003: TimelineController sorted_clips vector linear find in moveGroup
|
||||
* kdenlive-0004: TimelineModel all_items list linear find in resize
|
||||
* kdenlive-0005: PreviewManager m_renderedChunks/m_dirtyChunks QVariantList linear contains
|
||||
* kdenlive-0006: TimelineController canceled guides vector linear find
|
||||
* kdenlive-0007: AssetParameterModel m_rows indexOf in parameter loops
|
||||
* kdenlive-0008: UrlListParamWidget addItemsInSameFolder linear find on map values
|
||||
*/
|
||||
public class KdenliveTest {
|
||||
|
||||
|
|
@ -152,6 +156,145 @@ public class KdenliveTest {
|
|||
return ops;
|
||||
}
|
||||
|
||||
// --- kdenlive-0005: PreviewManager chunk lists linear contains ---
|
||||
|
||||
static long previewChunksDefect(int numDirty, int numNew) {
|
||||
// Simulates m_dirtyChunks as QVariantList with .contains() dedup
|
||||
List<Integer> dirtyChunks = new ArrayList<>();
|
||||
for (int i = 0; i < numDirty; i++) dirtyChunks.add(i * 25);
|
||||
List<Integer> renderedChunks = new ArrayList<>();
|
||||
for (int i = 0; i < numDirty / 2; i++) renderedChunks.add(i * 25);
|
||||
long ops = 0;
|
||||
// reloadChunks pattern: loop over new chunks, .contains() on existing
|
||||
for (int i = 0; i < numNew; i++) {
|
||||
int frame = i * 25;
|
||||
// m_dirtyChunks.contains(frame) - linear scan
|
||||
for (int existing : dirtyChunks) {
|
||||
ops++;
|
||||
if (existing == frame) break;
|
||||
}
|
||||
// m_renderedChunks.contains(frame) - linear scan
|
||||
for (int existing : renderedChunks) {
|
||||
ops++;
|
||||
if (existing == frame) break;
|
||||
}
|
||||
}
|
||||
return ops;
|
||||
}
|
||||
|
||||
static long previewChunksFixed(int numDirty, int numNew) {
|
||||
Set<Integer> dirtySet = new HashSet<>();
|
||||
for (int i = 0; i < numDirty; i++) dirtySet.add(i * 25);
|
||||
Set<Integer> renderedSet = new HashSet<>();
|
||||
for (int i = 0; i < numDirty / 2; i++) renderedSet.add(i * 25);
|
||||
long ops = 0;
|
||||
for (int i = 0; i < numNew; i++) {
|
||||
int frame = i * 25;
|
||||
ops++; // O(1) HashSet.contains for dirty
|
||||
dirtySet.contains(frame);
|
||||
ops++; // O(1) HashSet.contains for rendered
|
||||
renderedSet.contains(frame);
|
||||
}
|
||||
return ops;
|
||||
}
|
||||
|
||||
// --- kdenlive-0006: canceled guide filter ---
|
||||
|
||||
static long canceledGuidesDefect(int numGuides, int numCanceled) {
|
||||
List<Integer> guides = new ArrayList<>();
|
||||
for (int i = 0; i < numGuides; i++) guides.add(i * 30);
|
||||
List<Integer> canceled = new ArrayList<>();
|
||||
for (int i = 0; i < numCanceled; i++) canceled.add(i * 60); // every other guide
|
||||
long ops = 0;
|
||||
for (int guidePos : guides) {
|
||||
// std::find on canceled vector
|
||||
for (int c : canceled) {
|
||||
ops++;
|
||||
if (c == guidePos) break;
|
||||
}
|
||||
}
|
||||
return ops;
|
||||
}
|
||||
|
||||
static long canceledGuidesFixed(int numGuides, int numCanceled) {
|
||||
List<Integer> guides = new ArrayList<>();
|
||||
for (int i = 0; i < numGuides; i++) guides.add(i * 30);
|
||||
Set<Integer> canceledSet = new HashSet<>();
|
||||
for (int i = 0; i < numCanceled; i++) canceledSet.add(i * 60);
|
||||
long ops = 0;
|
||||
for (int guidePos : guides) {
|
||||
ops++; // O(1) HashSet lookup
|
||||
canceledSet.contains(guidePos);
|
||||
}
|
||||
return ops;
|
||||
}
|
||||
|
||||
// --- kdenlive-0007: AssetParameterModel m_rows indexOf in loops ---
|
||||
|
||||
static long rowsIndexOfDefect(int numParams) {
|
||||
// m_rows is QVector<QString>, indexOf called per param
|
||||
List<String> rows = new ArrayList<>();
|
||||
for (int i = 0; i < numParams; i++) rows.add("param_" + i);
|
||||
long ops = 0;
|
||||
// Simulates getAllParameters/toJson loop
|
||||
for (int i = 0; i < numParams; i++) {
|
||||
String name = "param_" + i;
|
||||
// indexOf: linear scan of m_rows
|
||||
for (int j = 0; j < rows.size(); j++) {
|
||||
ops++;
|
||||
if (rows.get(j).equals(name)) break;
|
||||
}
|
||||
}
|
||||
return ops;
|
||||
}
|
||||
|
||||
static long rowsIndexOfFixed(int numParams) {
|
||||
Map<String, Integer> rowIndex = new HashMap<>();
|
||||
for (int i = 0; i < numParams; i++) rowIndex.put("param_" + i, i);
|
||||
long ops = 0;
|
||||
for (int i = 0; i < numParams; i++) {
|
||||
ops++; // O(1) HashMap lookup
|
||||
rowIndex.get("param_" + i);
|
||||
}
|
||||
return ops;
|
||||
}
|
||||
|
||||
// --- kdenlive-0008: UrlListParamWidget addItemsInSameFolder ---
|
||||
|
||||
static long urlListFindDefect(int numEntries, int numExisting) {
|
||||
// QMap values iterated with std::find for each directory entry
|
||||
Map<String, String> listValues = new LinkedHashMap<>();
|
||||
for (int i = 0; i < numExisting; i++) {
|
||||
listValues.put("file_" + i, "/path/to/file_" + i + ".png");
|
||||
}
|
||||
long ops = 0;
|
||||
for (int i = 0; i < numEntries; i++) {
|
||||
String path = "/dir/entry_" + i + ".png";
|
||||
// std::find iterates all map values
|
||||
for (String val : listValues.values()) {
|
||||
ops++;
|
||||
if (val.equals(path)) break;
|
||||
}
|
||||
}
|
||||
return ops;
|
||||
}
|
||||
|
||||
static long urlListFindFixed(int numEntries, int numExisting) {
|
||||
Map<String, String> listValues = new LinkedHashMap<>();
|
||||
Set<String> existingValues = new HashSet<>();
|
||||
for (int i = 0; i < numExisting; i++) {
|
||||
String path = "/path/to/file_" + i + ".png";
|
||||
listValues.put("file_" + i, path);
|
||||
existingValues.add(path);
|
||||
}
|
||||
long ops = 0;
|
||||
for (int i = 0; i < numEntries; i++) {
|
||||
ops++; // O(1) HashSet lookup
|
||||
existingValues.contains("/dir/entry_" + i + ".png");
|
||||
}
|
||||
return ops;
|
||||
}
|
||||
|
||||
public static void main(String[] args) {
|
||||
int pass = 0, fail = 0;
|
||||
|
||||
|
|
@ -199,6 +342,50 @@ public class KdenliveTest {
|
|||
if (ok) pass++; else fail++;
|
||||
}
|
||||
|
||||
// Test kdenlive-0005: PreviewManager chunks (500 dirty, 500 new)
|
||||
{
|
||||
long defectOps = previewChunksDefect(500, 500);
|
||||
long fixedOps = previewChunksFixed(500, 500);
|
||||
double ratio = (double) defectOps / fixedOps;
|
||||
boolean ok = ratio > 50.0;
|
||||
System.out.printf("kdenlive-0005 PreviewManager chunk contains: defect=%d fixed=%d ratio=%.1fx %s%n",
|
||||
defectOps, fixedOps, ratio, ok ? "PASS" : "FAIL");
|
||||
if (ok) pass++; else fail++;
|
||||
}
|
||||
|
||||
// Test kdenlive-0006: canceled guides (500 guides, 200 canceled)
|
||||
{
|
||||
long defectOps = canceledGuidesDefect(500, 200);
|
||||
long fixedOps = canceledGuidesFixed(500, 200);
|
||||
double ratio = (double) defectOps / fixedOps;
|
||||
boolean ok = ratio > 50.0;
|
||||
System.out.printf("kdenlive-0006 canceled guides linear find: defect=%d fixed=%d ratio=%.1fx %s%n",
|
||||
defectOps, fixedOps, ratio, ok ? "PASS" : "FAIL");
|
||||
if (ok) pass++; else fail++;
|
||||
}
|
||||
|
||||
// Test kdenlive-0007: m_rows indexOf (50 params)
|
||||
{
|
||||
long defectOps = rowsIndexOfDefect(50);
|
||||
long fixedOps = rowsIndexOfFixed(50);
|
||||
double ratio = (double) defectOps / fixedOps;
|
||||
boolean ok = ratio > 10.0;
|
||||
System.out.printf("kdenlive-0007 m_rows indexOf in loop: defect=%d fixed=%d ratio=%.1fx %s%n",
|
||||
defectOps, fixedOps, ratio, ok ? "PASS" : "FAIL");
|
||||
if (ok) pass++; else fail++;
|
||||
}
|
||||
|
||||
// Test kdenlive-0008: urllist addItemsInSameFolder (300 entries, 200 existing)
|
||||
{
|
||||
long defectOps = urlListFindDefect(300, 200);
|
||||
long fixedOps = urlListFindFixed(300, 200);
|
||||
double ratio = (double) defectOps / fixedOps;
|
||||
boolean ok = ratio > 50.0;
|
||||
System.out.printf("kdenlive-0008 urllist value find: defect=%d fixed=%d ratio=%.1fx %s%n",
|
||||
defectOps, fixedOps, ratio, ok ? "PASS" : "FAIL");
|
||||
if (ok) pass++; else fail++;
|
||||
}
|
||||
|
||||
System.out.printf("%nSummary: %d/%d PASS%n", pass, pass + fail);
|
||||
if (fail > 0) System.exit(1);
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue