Opened 15 years ago

Closed 14 years ago

Last modified 12 years ago

#3183 closed defect (fixed)

[PATCH] Proposed fixes for wktraster

Reported by: Even Rouault Owned by: jorgearevalo
Priority: normal Milestone:
Component: GDAL_Raster Version: unspecified
Severity: normal Keywords: postgis_raster
Cc: warmerdam

Description

Jorge,

You'll find attached a patch with a bunch of changes from a first review :

  • CPLMalloc()/CPLCalloc() fails with an abort(). As you always check for the return value, you'd better use VSIMalloc()/VSICalloc() that fails nicely with the NULL pointer.
  • I've changed almost all the uses 'char szCommand[1024]' to CPLString osCommand to avoid potential stack buffer overflows with long strings (side note: you don't need to memset a buffer before sprint'ing in it)
  • you don't need to test if the pointer is non-NULL to call CPLFree() on it
  • I've modified the constructor of WKTRasterWrapper() to just initialize the variables freed in the destructor and introduced a Initialize() to do the real job. That way we can safely return from the Initialize() in error code paths and destruct the object in calling code if the return value of Initialize() is FALSE. Basically in C++, a constructor must always be successfull otherwise you end up easily with a half initialized object that can't be destroyed safely.
  • 32BF and 64BF were mapped to complex data types. I think this is wrong ? So I've mapped to GDT_Float32 and GDT_Float64 instead.
  • In Initialize(), I've added a test to check that there are enough bytes in the buffer to decode the header. Some others could be added for the band data. This is to protect against potential wrong data coming from the server.

All of this is untested apart from successfull compilation (compiling wktraster on win32 is a bit painful). So please double check and test before applying.

There are also 2 important FIXME in wktrasterwrapper.cpp that probably require fixes from you as far as nodata reading/setting is concerned.

Attachments (1)

wktraster_ticket3183.patch (35.8 KB ) - added by Even Rouault 15 years ago.

Download all attachments as: .zip

Change History (4)

by Even Rouault, 15 years ago

Attachment: wktraster_ticket3183.patch added

comment:1 by warmerdam, 15 years ago

Cc: warmerdam added
Component: defaultGDAL_Raster
Keywords: wktraster added

comment:2 by Even Rouault, 14 years ago

Resolution: fixed
Status: newclosed

Applied by Jorge in r17893

comment:3 by pracine, 12 years ago

Keywords: postgis_raster added; wktraster removed
Note: See TracTickets for help on using tickets.