DELETE_ARRAY in /inkbugs/inkscape/src/trace/quantize.cpp

Bug #613723 reported by Vaughn Spurlin on 2010-08-05
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Inkscape
Medium
Jon A. Cruz

Bug Description

DELETE_ARRAY in /inkbugs/inkscape/src/trace/quantize.cpp

In rgbMapQuantize(RgbMap_def *, int…):
Using non-array delete on an array of objects; should be using delete[] (CWE-459).

  557
Using new in : "new RGB[ncolor]".
Assigning: "rgbpal" = storage from "new RGB[ncolor]".
  558 RGB *rgbpal = new RGB[ncolor];
  559 int indexes = 0;
  560 octreeIndex(tree, rgbpal, &indexes);
  561
  562 octreeDelete(&pool, tree);
  563
  564 // stacking with increasing contrasts
  565 qsort((void *)rgbpal, indexes, sizeof(RGB), compRGB);
  566
  567 // make the new map
  568 IndexedMap *newmap = IndexedMapCreate(rgbmap->width, rgbmap->height);
At conditional (1): "!newmap" taking the true branch.
Deleting array variable "rgbpal" with non-array delete in "delete rgbpal".
  569 if (!newmap) { delete rgbpal; return NULL; }
...
  585
Deleting array variable "rgbpal" with non-array delete in "delete rgbpal".
  586 delete rgbpal;

Vaughn Spurlin (vspurlin) wrote :

fix suggestions 2010-07-25:
  569 if (!newmap) { delete[] rgbpal; return NULL; }
  586 delete[] rgbpal;

fix reason:
  delete only releases the first element of an array; delete[] releases the entire array.

Jon A. Cruz (jon-joncruz) wrote :

Delete correction is generally good, but in this case line 569 exhibits several problems.

First, multiple statements should not be collapsed to a single line. Second is that early returns should be avoided. Third actually comes from earlier in the function where rgbpal should probably not be new'd to begin with.

Changed in inkscape:
assignee: nobody → Jon A. Cruz (jon-joncruz)
Jon A. Cruz (jon-joncruz) wrote :

For informational purposes, I'm attaching a patch of the changes to fix this.

Note that due to the use of qsort, I'm not yet switching away from new[]. Doing so will require a bit of time for proper performance impact measurements.

Changed in inkscape:
milestone: none → 0.48
Changed in inkscape:
status: New → In Progress
status: In Progress → Fix Committed
jazzynico (jazzynico) on 2010-08-24
Changed in inkscape:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers