Opened 18 years ago

Last modified 18 years ago

#1146 closed defect (fixed)

OGRPGDatasource::FetchSrsId doesn't create proj4text and leaks memory

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

Description

The OGRPGDatasource::FetchSrsId method looks up the srsid in the postgis 
spatial_ref_sys table, and creates a new entry if it doesn't find an entry 
that matches the SRS being looked up.  In the current form, when it creates 
the entry it inserts the SRID and the WKT but not the PROJ4 string.  This 
submissions adds support for the PROJ4 string.

The method also calls OGRSpatialReference::exportToWKT and now 
OGRSpatialReference::exportToProj4 but didn't call CPLFree on the strings that 
were allocated data during these calls.  This submission also adds support for 
calling CPLFree.  Note that the easiest way to do this (that is touching the 
least amount of code) was to call exportToWKT, use it to look up the SRID, 
then free it.  Later in the code when the WKT is needed again, I made a second 
call to exportToWKT w/ a matching CPLFree.  The code had many return 
statements in the middle of the method.  This approach made it possible to add 
the appropriate CPLFree statements without restructuring the code to have a 
single exit point where everything could be freed.

Here is the updated method (My code has comments that begin with CM:) :

/************************************************************************/
/*                             FetchSRSId()                             */
/*                                                                      */
/*      Fetch the id corresponding to an SRS, and if not found, add     */
/*      it to the table.                                                */
/************************************************************************/

int OGRPGDataSource::FetchSRSId( OGRSpatialReference * poSRS )

{
    PGresult            *hResult;
    char                szCommand[10000];
    char                *pszWKT = NULL;
    int                 nSRSId;

    if( poSRS == NULL )
        return -1;

/* -------------------------------------------------------------------- */
/*      Translate SRS to WKT.                                           */
/* -------------------------------------------------------------------- */
    if( poSRS->exportToWkt( &pszWKT ) != OGRERR_NONE )
        return -1;

    CPLAssert( strlen(pszWKT) < sizeof(szCommand) - 500 );


/* -------------------------------------------------------------------- */
/*      Try to find in the existing table.                              */
/* -------------------------------------------------------------------- */
    hResult = PQexec(hPGConn, "BEGIN");

    sprintf( szCommand,
             "SELECT srid FROM spatial_ref_sys WHERE srtext = '%s'",
             pszWKT );
    hResult = PQexec(hPGConn, szCommand );
	CPLFree( pszWKT );  // CM:  Added to prevent mem leaks
	pszWKT = NULL;      // CM:  Added

/* -------------------------------------------------------------------- */
/*      We got it!  Return it.                                          */
/* -------------------------------------------------------------------- */
    if( hResult && PQresultStatus(hResult) == PGRES_TUPLES_OK
        && PQntuples(hResult) > 0 )
    {
        nSRSId = atoi(PQgetvalue( hResult, 0, 0 ));

        PQclear( hResult );

        hResult = PQexec(hPGConn, "COMMIT");
        PQclear( hResult );

        return nSRSId;
    }

/* -------------------------------------------------------------------- */
/*      If the command actually failed, then the metadata table is      */
/*      likely missing. Try defining it.                                */
/* -------------------------------------------------------------------- */
    int         bTableMissing;

    bTableMissing =
        hResult == NULL || PQresultStatus(hResult) == PGRES_NONFATAL_ERROR;

    hResult = PQexec(hPGConn, "COMMIT");
    PQclear( hResult );

    if( bTableMissing )
    {
        if( InitializeMetadataTables() != OGRERR_NONE )
            return -1;
    }

/* -------------------------------------------------------------------- */
/*      Get the current maximum srid in the srs table.                  */
/* -------------------------------------------------------------------- */
    hResult = PQexec(hPGConn, "BEGIN");
    PQclear( hResult );

    hResult = PQexec(hPGConn, "SELECT MAX(srid) FROM spatial_ref_sys" );

    if( hResult && PQresultStatus(hResult) == PGRES_TUPLES_OK )
    {
        nSRSId = atoi(PQgetvalue(hResult,0,0)) + 1;
        PQclear( hResult );
    }
    else
        nSRSId = 1;

/* -------------------------------------------------------------------- */
/*      Try adding the SRS to the SRS table.                            */
/* -------------------------------------------------------------------- */
	// CM:  Added pszProj4 acquisition and appended it to SQL insert.
	if( poSRS->exportToWkt( &pszWKT ) != OGRERR_NONE )
	{
		return -1;
	}

	CPLAssert( strlen(pszWKT) < sizeof(szCommand) - 500 );

	char    *pszProj4 = NULL;
	if( poSRS->exportToProj4( &pszProj4 ) != OGRERR_NONE )
	{
		CPLFree( pszWKT );  // Prevent mem leaks
		pszWKT = NULL;
		
		return -1;
	}

	sprintf( szCommand,
             "INSERT INTO spatial_ref_sys (srid,srtext,proj4text) VALUES (%d,'%
s','%s')",
             nSRSId, pszWKT, pszProj4 );
	
	// Free everything that was allocated.
	CPLFree( pszProj4 );
    CPLFree( pszWKT);

	// CM:  End my changes.

    hResult = PQexec(hPGConn, szCommand );
    PQclear( hResult );

    hResult = PQexec(hPGConn, "COMMIT");
    PQclear( hResult );

    return nSRSId;
}

Attachments (1)

FetchSrsId.cpp (4.3 KB ) - added by craig.miller@… 18 years ago.
FetchSrsId method in a file

Download all attachments as: .zip

Change History (2)

by craig.miller@…, 18 years ago

Attachment: FetchSrsId.cpp added

FetchSrsId method in a file

comment:1 by warmerdam, 18 years ago

Craig,

I have checked and this seems to work properly.  Change committed. 

Thanks!

Note: See TracTickets for help on using tickets.