Opened 6 years ago

Closed 6 years ago

#3897 closed defect (fixed)

GDALRenameDataset on HFA does not update external spill reference

Reported by: rprinceley Owned by: warmerdam
Priority: normal Milestone: 1.8.0
Component: GDAL_Raster Version: svn-trunk
Severity: normal Keywords: HFA
Cc: gaopeng

Description

When renaming an HFA dataset with external spill file, the new dataset maintains reference to old .ige file that no longer exists.

GDALCopyDatasetFiles behaves similarly - creates new copies of .ige and .rrd but dataset uses original files.

Change History (12)

comment:1 Changed 6 years ago by warmerdam

Keywords: gtiff added
Status: newassigned

comment:2 Changed 6 years ago by warmerdam

Keywords: HFA added; gtiff removed
Milestone: 1.8.0

I have provided a custom implementation of CopyFiles? and Rename for the HFA driver which knows how to update RRDNamesList and the .ige reference in trunk (r21432). r21431 and r21428 were also required to get this working.

I'm leaving this ticket open pending of adding of regression tests for this capability.

Robin, do you need this backported to 1.6-esri? I am concerned it is a bit risky.

comment:3 Changed 6 years ago by warmerdam

Note, r21433 making CPLCorrespondingPaths() was also needed for some situations, particularly when a .aux.xml file existed.

comment:4 Changed 6 years ago by rprinceley

Please back port to 1.6-esri, this is something reported my several users.

comment:5 Changed 6 years ago by rprinceley

Frank, could you please update HFABand::LoadExternalBlockInfo?() to use HFAGetIGEFilename().

We also need to handle badly renamed files gracefully, will adding a VSIStat() check and falling back to using basename in HFAGetIGEFilename() be alright?

comment:6 Changed 6 years ago by warmerdam

Resolution: fixed
Status: assignedclosed

I have significantly reworked the rename/copy logic to work even when there is an .rrd file with a .rde spill file. Added logic in HFAGetIGEFilename() so it falls back to looking for a file with the same basename as the .img file. As suggested I have updated LoadExternalBlockInfo?() to use this improved HFAGetIGEFilename(). All done in trunk (r21476).

I also fixed breakage in CPLMoveFile() (r21474) which made renaming large files expensive due to a fallback to copy/delete.

This work has all been backported into 1.6-esri (r21477).

comment:7 Changed 6 years ago by warmerdam

Added regression test on complex hfa renaming (r21478).

comment:8 Changed 6 years ago by rprinceley

Resolution: fixed
Status: closedreopened

Frank,

There is a crash in HFARenameReferences when you rename a file multiple times (GetStringField?() returns NULL).

        for( i = 0; i < nNameCount; i++ )
        {
            CPLString osFN;
            osFN.Printf( "nameList[%d].string", i );
            aosNL.push_back( poRRDNL->GetStringField(osFN) );
        } 

We were able to trigger this by increasing the length first then reducing it (exact strings we used: "malaybalayortho_cr.img" -> "malaybalayortho_cr_1.img" -> "test.img").

comment:9 Changed 6 years ago by warmerdam

Problem confirmed. There is a problem with the logic circa line 426 of hfafield.cpp when the data array has existing random garbage in it. I will come up with a fix soon.

comment:10 Changed 6 years ago by warmerdam

Resolution: fixed
Status: reopenedclosed

I have applied what I think it is a specific and safe fix for the problem in HFARenameDependencies() (by reinitializing the data segment to all zeros and completely reconstructing it). This is applied in trunk (r21575, r21572), 1.8 (r21576,21573), and 1.6-esri (r21577, r21574).

I believe this fixes the issue at hand.

comment:11 Changed 6 years ago by rprinceley

Resolution: fixed
Status: closedreopened

There is a minor issue in cpl_path.cpp (r21433) that causes CPLCorrespondingPaths() to fail if aux.xml file (or other file with different basename) is present in the file list.

            if( !EQUALN(papszFileList[i],osOldBasename,osOldBasename.size())
                || papszFileList[i][osOldBasename.size()] != '.' )

papszFileList[i] contains full path not basename.

comment:12 Changed 6 years ago by warmerdam

Resolution: fixed
Status: reopenedclosed

I have applied a patch to take into account the path which seems to work properly (r21583, r21584, r21585).

Let me know if problems remain.

Note: See TracTickets for help on using tickets.