Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#5363 closed defect (fixed)

crash in GDALAutoCreateWarpedVRT via recent change

Reported by: vindoctor Owned by: Kyle Shannon
Priority: highest Milestone:
Component: GDAL_Raster Version: svn-trunk
Severity: blocker Keywords: warper crash null pointer
Cc: Kyle Shannon

Description

file vrtwarped.cpp

location: /* -------------------------------------------------------------------- */ /* Define band mapping if necessary. */ /* -------------------------------------------------------------------- */ someone added some if statements which causes memory to not get created!

with the logic they have, this if statement will run without memory being created:

for( i = 0; i < psWO->nBandCount; i++ ) {

psWO->panSrcBands[i] = i+1; psWO->panDstBands[i] = i+1;

}

I've removed the if statement as I don't see why it was added in the first place. I can now warp my tiffs again!

if( psWO->nBandCount == GDALGetRasterCount( hSrcDS )

psWO->nBandCount == 0 )

{

VAA removed

if( psWO->nBandCount == 0 ) {

psWO->nBandCount = GDALGetRasterCount( hSrcDS ); psWO->panSrcBands = (int *) CPLMalloc(sizeof(int) * psWO->nBandCount); psWO->panDstBands = (int *) CPLMalloc(sizeof(int) * psWO->nBandCount);

}

for( i = 0; i < psWO->nBandCount; i++ ) {

psWO->panSrcBands[i] = i+1; psWO->panDstBands[i] = i+1;

}

}

Change History (7)

comment:1 by Even Rouault, 10 years ago

Can you provide a self-contained code that triggers the crash ?

comment:2 by Even Rouault, 10 years ago

Cc: Kyle Shannon added

Kyle, I believe the above report relates to r26121, #5138

comment:3 by Kyle Shannon, 10 years ago

Hi, Are you setting psWO->nBandCount? I see that the check for:

psWO->nBandCount == GDALGetRasterCount(hSrcDS)

Shouldn't be there, but if you set psWO->nBandCount, band maps are assumed to be there. I will remove the check.

Is it possible to post your code?

comment:4 by Kyle Shannon, 10 years ago

Owner: changed from warmerdam to Kyle Shannon

I am assuming that you are setting psWO->nBandCount to all of the bands, but not creating the associated band maps. In the past, AutoCreateVRT ignored most (if not all) warp options and just created default band maps, 1..nBands for psWO->panSrcBands and psWO->panDstBands. The second check mentioned above probably shouldn't be there, but by setting psWO->nBandCount, most warp operations assume that the band map(s) have been allocated and defined. Either way, my change has made AutoCreateVRT less 'automatic', so I'm inclined to revert the previous change instead of breaking the assumption that AutoCreateVRT just 'works' in most cases. If I have made any incorrect assumptions, please reply. I will revert r26121 shortly.

comment:5 by Kyle Shannon, 10 years ago

Resolution: fixed
Status: newclosed

Partially reverted r26121 in trunk r26891

comment:6 by Kyle Shannon, 10 years ago

vindoctor, For documentation sake, could you answer the questions regarding the warp options above? It would be helpful, thanks.

in reply to:  6 comment:7 by vindoctor, 10 years ago

Replying to kyle:

vindoctor, For documentation sake, could you answer the questions regarding the warp options above? It would be helpful, thanks.

Correct as to make it just work keeps the same behavior and I would not have to allocate things outside the auto create. A good rule I use when wanting a new behavior in an API behavior is, I make a new method/API and mark the previous to be obsoleted in a future version, giving developers a chance to upgrade. Or just comment out the API with a comment of the new API and how to use it with the new behavior. You and your group are doing a great job and thank you for all your help!

"so I'm inclined to revert the previous change instead of breaking the assumption that AutoCreateVRT just 'works' in most cases. If I have made any incorrect assumptions, please reply. I will revert r26121 shortly."

Note: See TracTickets for help on using tickets.