FORWARD_NULL in /inkbugs/inkscape/src/libavoid/graph.cpp

[Vaughn Spurlin]

FORWARD_NULL in /inkbugs/inkscape/src/libavoid/graph.cpp

In Avoid::?EdgeInf…::?rotationLessThan(...):
Dereference of an explicit null value (CWE-476).

Assigning: "lhsV" = 0.
  137 VertInf *lhsV = NULL, *rhsV = NULL, *commonV = NULL;
  138
  139 // Determine common Point and the comparison point on the left- and
  140 // the right-hand-side.
At conditional (1): "this->_v1 == rhs->_v1" taking the false branch.
  141 if (_v1 == rhs->_v1)
  142 {
  143 commonV = _v1;
  144 lhsV = _v2;
  145 rhsV = rhs->_v2;
  146 }
At conditional (2): "this->_v1 == rhs->_v2" taking the false branch.
  147 else if (_v1 == rhs->_v2)
  148 {
  149 commonV = _v1;
  150 lhsV = _v2;
  151 rhsV = rhs->_v1;
  152 }
At conditional (3): "this->_v2 == rhs->_v1" taking the false branch.
  153 else if (_v2 == rhs->_v1)
  154 {
  155 commonV = _v2;
  156 lhsV = _v1;
  157 rhsV = rhs->_v2;
  158 }
At conditional (4): "this->_v2 == rhs->_v2" taking the false branch.
  159 else if (_v2 == rhs->_v2)
  160 {
  161 commonV = _v2;
  162 lhsV = _v1;
  163 rhsV = rhs->_v1;
  164 }
  165
Assigning null: "lhsPt" = "&lhsV->point".
  166 const Point& lhsPt = lhsV->point;

[Vaughn Spurlin (vspurlin)]

fix suggestion 2010-07-25:
  164 } else
  164.1 {
  164.2 return false;
  164.3 }
  165

fix reason:
  This method assumes two Edges that share a common point.
  If the edges coincide, the method exits immediately.
  The subsequent if-else logic determines what end point the edges share and sets lhsV to that point.
  If no shared end point is found, the edges do not touch.
  Hence, the method should exit, as it did when the edges coincided.
  Exiting prevents dereferencing lhsV if it was not set to an end point.

[Jon A. Cruz (jon-joncruz)]

Extra early returns should be avoided. The code can be structured to correct without the complexity of multiple exit points.