Changes between Initial Version and Version 2 of Ticket #81


Ignore:
Timestamp:
Dec 22, 2007, 1:31:18 AM (16 years ago)
Author:
mloskot
Comment:

Legend:

Unmodified
Added
Removed
Modified

  • Ticket #81

    • Property Reporter changed from mateusz@… to mloskot
    • Property Priority 5 → major
    • Property Version 3.0.0 → svn-trunk
    • Property Milestone imported → 3.0.0
    • Property Resolution none → fixed

  • Ticket #81 – Description

    initial v2  
     1Today, when I was woring on Unit Tests I found a pretty serious bug causing memory access violation error. Here is the story:
    12{{{
    2 Today, when I was woring on Unit Tests I found a pretty serious bug causing memory access violation error. Here is the story:
    3 
    43// Factory object preparation
    54const int srid = 1;
    … …  
    1211// ring is empty, but let's see what will happen
    1312PointPtr x = ring->getStartPoint(); <--- BUM!
     13}}}
    1414
    1515The line marked with BUM! causes memory access violation error.
    … …  
    1717Here is descriptive backtrace with complete call chain (lines marked with <--- are executed during step-by-step debugging):
    1818
    19 1.
    20 
     19 1.
     20{{{
    2121PointPtr x = ring->getStartPoint(); <---
    22 
    23 1.1.
    24 
     22}}}
     23 1.1.
     24{{{
    2525Point* LineString::getStartPoint() const {
    2626   if (isEmpty()) {
    … …  
    2929   return getPointN(0);
    3030}
     31}}}
     32 2.
     33{{{
     34Point(NULL, NULL); <---
     35}}}
    3136
    32 2. Point(NULL, NULL); <---
     37 Parameters:
     38  * newCoord = NULL
     39  * factory = NULL
    3340
    34 Parameters:
    35 newCoord = NULL
    36 factory = NULL
    37 
    38 2.1.
    39 
     41 2.1.
     42{{{
    4043Point::Point(CoordinateSequence *newCoords, const GeometryFactory *factory)
    4144: Geometry(factory) <---
    … …  
    4346//...
    4447}
     48}}}
    4549
    46 3.
    47 
     50 3.
     51{{{
    4852Geometry(factory)
    4953
    5054factory = NULL
     55}}}
    5156
    52 3.1.
    53 
     57 3.1.
     58{{{
    5459Geometry::Geometry(const GeometryFactory* newFactory)
    5560{
    … …  
    5964   userData=NULL;
    6065}
     66}}}
    6167
    62 [1]
    63 newFactory = NULL, so factory gets becomes null pointer
     68 * (1) newFactory = NULL, so factory gets becomes null pointer
    6469
    65 [2] BUUUM!
    66 factory is a null pointer and calling member on a null pointer value causes undefined behaviour!
     70 * (2) BUUUM! factory is a null pointer and calling member on a null pointer value causes undefined behaviour!
    6771
     72{{{
    6873SRID=factory->getSRID();
     74}}}
    6975
    70 4. Finally, GeometryFactory::getSRID() throws
    71    MEMORY ACCESS VIOLATION ERROR!
     76 4. Finally, GeometryFactory::getSRID() throws '''memory access violation error'''
    7277
    7378
    7479Summary:
    75 - the Geometry constructor is used incorrectly
    76 - or this constructor is incorrectly designed to handle all valid cases
    77 - This is another example of ***JAVISMS***.
     80 * the Geometry constructor is used incorrectly
     81 * or this constructor is incorrectly designed to handle all valid cases
     82 * This is another example of ''JAVISMS''.
    7883
    7984JTS version of getStartPoint is defined as follows:
    8085
     86{{{
    8187public Point getStartPoint() {
    8288   if (isEmpty()) {
    … …  
    8591   return getPointN(0);
    8692}
     93}}}
    8794
    88 [3] returning null reference is completely different semantic of returning pointer to newly allocated object, what happens in GEOS' version of getStartPoint!!!
     95 * (3) returning null reference is completely different semantic of returning pointer to newly allocated object, what happens in GEOS' version of getStartPoint!
    8996
    9097GEOS' version of getStartPoint is buggy in two ways:
    91 1. Causes memory access violation
    92 2. There is logical bug: how can user expect to get Point object returned from *empty* LinearRing??? Such behaviour does not make sense.
    93 It's also, what is most important, incompatible behaviour with that in JTS version!
     98
     99 1. Causes memory access violation
     100 2. There is logical bug: how can user expect to get Point object returned from *empty* LinearRing??? Such behaviour does not make sense. It's also, what is most important, incompatible behaviour with that in JTS version!
    94101
    95102I'm quite confused and I'm really affraid of possible bugs hidding in GEOS. Please, discuss it in details and let's make this software stable!
    96 }}}