Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#2224 closed defect (fixed)

GSAG (Golden Surfer - ASCII) Fixes

Reported by: warmerdam Owned by: warmerdam
Priority: normal Milestone: 1.5.1
Component: GDAL_Raster Version: svn-trunk
Severity: normal Keywords: GSAG
Cc: reedc@…, Even Rouault, dron, kwl7@…

Description

Frank,

I've attached a fix for the Golden Surfer File ascii dataset (GSAG driver). This was done off the trunk version.

As far as I can tell, it looks like all 3 surfer formats are working properly now, both for reading and writing (in the two that allow writing). I looked at bug #1616 - it looks like hte problem was writing ascii files, but not writing binary (GSAG was bad, GSBG was good). With this fix, it seems okay.

I made a couple of decisions with this - First, for reading, I took option 1 (where I actually end up reading the file 2x if needed, although it's done as needed, not on "Open"). Second - for writing, I fixed CreateCopy?(), and removed the Create() method. The Create() method was actually crashing, and didn't make much sense for an ascii file driver that needs to have the data in place since it's written "upside down" when compared with other GDAL formats. This seemed to make a lot more sense to me.

Please let me know if you need anything else to get this into the system. This will fix bugs #2191 and (I believe) also fixes #1616.

Thanks, Reed Copsey

Attachments (1)

gsagdataset.cpp (49.9 KB) - added by warmerdam 12 years ago.
Updated code for GSAG Driver

Download all attachments as: .zip

Change History (5)

Changed 12 years ago by warmerdam

Attachment: gsagdataset.cpp added

Updated code for GSAG Driver

comment:1 Changed 12 years ago by warmerdam

Cc: reedc@… Even Rouault dron kwl7@… added
Milestone: 1.6.0
Resolution: fixed
Status: newclosed

Reed,

I have applied your version in trunk (r13776) while trying to merge in a few changes Even made since you started work. I would appreciate your and/or Even reviewing the changes. Please close #1616 and #2191 if with appropriate references to this ticket and r13776 if you are comfortable they are dealt with. I'd also appreciate advice on whether the trunk version ought to be retrofit into 1.5 branch if it is safe and fixes lots of problems.

... late breaking news. Extra r13778 applied to fix an array underrun bug, and some reformatting. And r13777 applied to correct the test suite gsg.py script for GSAG.

comment:2 Changed 12 years ago by Even Rouault

This looks OK for me as far as my previous fixes are still there. I've just added a few additionnal checks on nRasterXSize and nRasterYSize, so that bogus files don't make GDAL crash in r13783.

I've also re-enabled gsg_5 autotest which runs OK after having updated the checksum to the checksum updated in r13777.

As far as backporting into 1.5 branch, I guess it's OK to do it. People will probably be happy to see the grid upside up ;-). But, I have no particular strong personal opinion on the subject. I was basically interested on the security issues in the driver.

comment:3 Changed 12 years ago by ReedC

This works perfectly for all of my test cases. I've closed #1616 and #2191, as well, since it appears to have resolved both of those issues.

comment:4 Changed 12 years ago by Even Rouault

Milestone: 1.6.01.5.1

The latest trunk version (revision r13783) has been backported in branches/1.5 in r13829 (this has implied a trivial change of VSIMalloc2(x, y) to VSIMalloc(x * y)). We should also note that the backported version has no longer any ::Create method. The existing implementation was buggy and it was considered almost impossible to implement random write access into an ASCII file, thus the removal.

Note: See TracTickets for help on using tickets.