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

[Vaughn Spurlin]

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)]

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)]

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.

[Jon A. Cruz (jon-joncruz)]

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.