#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 , 10 years ago
comment:3 by , 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 , 10 years ago
Owner: | changed from | to
---|
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 , 10 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
follow-up: 7 comment:6 by , 10 years ago
vindoctor, For documentation sake, could you answer the questions regarding the warp options above? It would be helpful, thanks.
comment:7 by , 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."
Can you provide a self-contained code that triggers the crash ?