[Gdal-dev] importFromWkt() problem

[Mateusz Loskot]

Petteri Packalen wrote:
> Mateusz, Frank, list,
> 
> I prepared the code below that demonstrates an essential part of the
> function in question. There was actually a bug in my first mail in
> example 1. Here is the more comprehensive illustration. Thus, although
> in both examples the address of the COPY of the pointer is passed to
> createFromWkt() or importFromWkt() only latter one works properly.
> 
> // Some variables needed in the above code.
> OGRFeature   *poSrcFeature;
> char         *geo_wkt = new char[ WKT_BUF ];
> char         *geo_trans = new char[ WKT_BUF ];
> 
> // This loop which goes through all OGR features. Partly taken
> // from the TranslateLayer() function in ogr2ogr.cpp.
> while( TRUE )
> {
>    OGRFeature *poDstFeature = NULL;
> 
>    // Fetch input feature.
>    poSrcFeature = poSrcLayer->GetNextFeature();
>    if ( poSrcFeature == NULL )
>       break;
> 
>    if ( ++nFeaturesInTransaction == nGroupTransactions )
>    {
>       poDstLayer->CommitTransaction();
>       poDstLayer->StartTransaction();
>       nFeaturesInTransaction = 0;
>    }
> 
>    // Create output feature based on input feature.
>    CPLErrorReset();
>    poDstFeature = OGRFeature::CreateFeature( poDstLayer->GetLayerDefn() );
>    poDstFeature->SetFrom( poSrcFeature, TRUE );
> 
>    // Fetch output geometry ref.
>    OGRGeometry *poGeom = poDstFeature->GetGeometryRef();
> 
>    // Export geometry to WKT to be transformed.
>    poGeom->exportToWkt( &geo_wkt );



If this is your complete set of operations, I don't see that you're
clearing the geo_trans buffer between filling it with exportToWkt().

So, you likely are getting garbage from previous WKT if the latter
is shorter than previous.

Here is small case study that presents my considerations:

//////////////////////////////////////////////////////////////////////
// Compile:
// g++ -Wall -pedantic -ansi -o newedchar newedchar.cpp

#include <iostream>
#include <cstring>
#include <cassert>

void print_data(const char* data, std::size_t size)
{
    assert(0 != data);

    std::cout << "------ START (size=" << size - 1 << ") ------\n";

    for (std::size_t i = 0; i != size - 1; ++i)
        std::cout << data[i] << " , ";

    std::cout << "\n------ END ------\n\n";
}

int main()
{
    // Short data source
    const std::size_t shortSize = 5;
    const char shortData[] = "12345"; // 5 elements + null

    // Long data source
    const std::size_t longSize = 8;
    const char longData[] = "abcdefgh"; // 8 elements + null

    // Destination buffer
    const std::size_t size = 15; // 14 elements + null
    char* dest = new char[size]; // filled with 0

    const char* p = 0; // iterator used to print, 'dest' stays unchanged

    // Copy long data to common buffer
    std::memcpy(dest, longData, longSize);

    p = dest;
    print_data(p, size);

    // DON'T CLEAN THE BUFFER (uncomment to clear buffer in next run)
    // std::memset(dest, 0, size);

    // Copy short data to common buffer
    std::memcpy(dest, shortData, shortSize);

    p = dest;
    print_data(dest, size);

    delete[] dest;

    return 0;
}
//////////////////////////////////////////////////////////////////////




And here is the output of this program:

mloskot:~$ ./bufftest
------ START (size=14) ------
a , b , c , d , e , f , g , h ,  ,  ,  ,  ,  ,  ,
------ END ------

------ START (size=14) ------
1 , 2 , 3 , 4 , 5 , f , g , h ,  ,  ,  ,  ,  ,  ,
------ END ------


The first buffer print is correct, it includes what we expect.
The second print is incorrect, it includes *garbage*, previous
content of buffer:

1 , 2 , 3 , 4 , 5 , f , g , h ,  ,  ,  ,  ,  ,  ,
-------------------^^^^^^^^^^^ - GARBAGE



But the correct and safe result should look as follows:

------ START (size=14) ------
a , b , c , d , e , f , g , h ,  ,  ,  ,  ,  ,  ,
------ END ------

------ START (size=14) ------
1 , 2 , 3 , 4 , 5 ,  ,  ,  ,  ,  ,  ,  ,  ,  ,
------ END ------


I hope it helps.

Cheers
-- 
Mateusz Loskot
http://mateusz.loskot.net