Opened 21 years ago

Last modified 20 years ago

#384 closed defect (fixed)

DIV by 0 in OGRLineString::Value

Reported by: sf@… Owned by: warmerdam
Priority: high Milestone:
Component: OGR_SF Version: unspecified
Severity: normal Keywords:
Cc:

Description

If OGRLineString::Value (calculates coords for a point on the line string at the 
given offset) is called for OGRLineString which have two identical succeeding 
points, a division by 0 occures.

Also it seems that inside the function the variable dfLength is not updated 
while the line segments are iterated.

Below is a diff against current CVS HEAD that fixes the missing bits.

Regards,
David Schalig

$ cvs diff ogrlinestring.cpp
Index: ogrlinestring.cpp
===================================================================
RCS file: /cvsroot/osrs/gdal/ogr/ogrlinestring.cpp,v
retrieving revision 1.38
diff -u -r1.38 ogrlinestring.cpp
--- ogrlinestring.cpp   9 Jun 2003 13:48:54 -0000       1.38
+++ ogrlinestring.cpp   26 Aug 2003 16:56:10 -0000
@@ -969,22 +969,27 @@
         dfDeltaY = paoPoints[i+1].y - paoPoints[i].y;
         dfSegLength = sqrt(dfDeltaX*dfDeltaX + dfDeltaY*dfDeltaY);

-        if( dfLength <= dfDistance && dfLength + dfSegLength >= dfDistance )
+        if (dfSegLength > 0)
         {
-            double      dfRatio;
+            if( (dfLength <= dfDistance) && ((dfLength + dfSegLength) >= 
dfDistance) )
+            {
+                double      dfRatio;

-            dfRatio = (dfDistance - dfLength) / dfSegLength;
+                dfRatio = (dfDistance - dfLength) / dfSegLength;

-            poPoint->setX( paoPoints[i].x * (1 - dfRatio)
-                           + paoPoints[i+1].x * dfRatio );
-            poPoint->setY( paoPoints[i].y * (1 - dfRatio)
-                           + paoPoints[i+1].y * dfRatio );
+                poPoint->setX( paoPoints[i].x * (1 - dfRatio)
+                               + paoPoints[i+1].x * dfRatio );
+                poPoint->setY( paoPoints[i].y * (1 - dfRatio)
+                               + paoPoints[i+1].y * dfRatio );

-            if( getCoordinateDimension() == 3 )
-                poPoint->setZ( padfZ[i] * (1 - dfRatio)
-                               + padfZ[i] * dfRatio );
+                if( getCoordinateDimension() == 3 )
+                    poPoint->setZ( padfZ[i] * (1 - dfRatio)
+                                   + padfZ[i] * dfRatio );

-            return;
+                return;
+            }
+
+            dfLength += dfSegLength;
         }
     }

Change History (6)

comment:1 by warmerdam, 21 years ago

David,

I was unable to directly apply the patch for some reason, and I am not confident
I will get it right by hand. Could you include a complete copy of your
modified Value() method? 

Thanks,

comment:2 by sf@…, 20 years ago

Any news on this bug? The current CVS HEAD still has it...
It would be very nice if it gets fixed.

comment:3 by warmerdam, 20 years ago

David, 

If you will read up in the bug report, I am waiting for the full text of
your implementation since I don't trust myself hand applying the patch.

Best regards,

comment:4 by sf@…, 20 years ago

Hello Frank,

sorry, I did not notice your reply. Here I send you my complete 
OGRLineString::Value function.

David

----------------------------------------
void OGRLineString::Value( double dfDistance, OGRPoint * poPoint )

{
    double      dfLength = 0;
    int         i;

    if( dfDistance < 0 )
    {
        StartPoint( poPoint );
        return;
    }

    for( i = 0; i < nPointCount-1; i++ )
    {
        double      dfDeltaX, dfDeltaY, dfSegLength;

        dfDeltaX = paoPoints[i+1].x - paoPoints[i].x;
        dfDeltaY = paoPoints[i+1].y - paoPoints[i].y;
        dfSegLength = sqrt(dfDeltaX*dfDeltaX + dfDeltaY*dfDeltaY);

        if (dfSegLength > 0)
        {
            if( (dfLength <= dfDistance) && ((dfLength + dfSegLength) >= 
dfDistance) )
            {
                double      dfRatio;

                dfRatio = (dfDistance - dfLength) / dfSegLength;

                poPoint->setX( paoPoints[i].x * (1 - dfRatio)
                               + paoPoints[i+1].x * dfRatio );
                poPoint->setY( paoPoints[i].y * (1 - dfRatio)
                               + paoPoints[i+1].y * dfRatio );

                if( getCoordinateDimension() == 3 )
                    poPoint->setZ( padfZ[i] * (1 - dfRatio)
                                   + padfZ[i] * dfRatio );
                
                return;
            }

            dfLength += dfSegLength;
        }
    }
    
    EndPoint( poPoint );
}

comment:5 by warmerdam, 20 years ago

This should be resolved before the 1.2.0 release.

comment:6 by warmerdam, 20 years ago

Modified version applied.  Not tested.

Note: See TracTickets for help on using tickets.