#3178 closed enhancement (fixed)
GDALDestroyWarpOptions to do nothing for null
Reported by: | Mateusz Łoskot | Owned by: | warmerdam |
---|---|---|---|
Priority: | normal | Milestone: | 2.0.0 |
Component: | GDAL_Raster | Version: | svn-trunk |
Severity: | normal | Keywords: | |
Cc: |
Description (last modified by )
GDALDestroyWarpOptions function should follow behaviour of std::free function and other standard C/C++ deallocators and should do nothing if nullptr is passed.
Currently, it throws error if psOptions is nullptr.
The change is trivial:
void CPL_STDCALL GDALDestroyWarpOptions( GDALWarpOptions *psOptions ) { if (psOptions) { // current definition } }
Actually, this should apply to all GDAL/OGR deallocators (Destroy and Close functions).
Change History (9)
comment:1 by , 15 years ago
Description: | modified (diff) |
---|
comment:2 by , 10 years ago
comment:3 by , 10 years ago
That is not an issue, I can provide the patch or commit it directly to the upstream repo. This issue has been opened as a request for comment and confirmation if I shall proceed or not.
Most of the tickets I opened as request for comments and which haven't been closed yet are waiting for final word from Frank or Even. So, please, do not close them w/o Frank's or Even's decision. Thanks.
comment:5 by , 10 years ago
Even,
I have reviewed current destroy/free-like functions and many of them either already include proposed changes and are no-op on nullptr input, or perform input pointer validation with the common macro. Some functions report CE_Failure, and others do not check input argument(s) at all.
Here is list of my findings:
GDALDestroyApproxTransformer: VALIDATE_POINTER0 GDALDestroyDriver: VALIDATE_POINTER0 GDALDestroyGCPTransformer: VALIDATE_POINTER0 GDALDestroyGenImgProjTransformer: VALIDATE_POINTER0 GDALDestroyGeoLocTransformer: no check, potential access violation GDALDestroyRPCTransformer: no check, potential access violation GDALDestroyReprojectionTransformer: VALIDATE_POINTER0 GDALDestroyTPSTransformer: VALIDATE_POINTER0 GDALDestroyTransformer: CE_Failure GDALDestroyWarpOptions: VALIDATE_POINTER0 CPLFree: already no-op GDALClose: already no-op GDALDestroyColorTable: already no-op GDALDestroyRasterAttributeTable: already no-op GDALDestroyScaledProgress: already no-op GDALDestroyWarpOperation: already no-op OGRFree: already no-op OGRGeocodeFreeResult: already no-op VSIFree: already no-op
- Do we want to replace
VALIDATE_POINTER0
with if nullptr do no-op strategy? I think it would be good to unify allGDALDestroy*
, as are those at the bottom of the list above.
- The two with no-check,
GDALDestroyGeoLocTransformer
andGDALDestroyRPCTransformer
, should be updated to either do VALIDATE_POINTER0 check or do no-op for nullptr input. Which one to go for?
- What to do with
GDALDestroyTransformer
?
comment:6 by , 10 years ago
I'd say to change all them to apply no-op strategy. Regarding GDALDestroyTransformer, it should be a no-op if receiving NULL, but still do the check for the GTI2 signature in the non null case.
comment:7 by , 9 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Finally, discussed changes applied in trunk @ r29207
comment:8 by , 9 years ago
Milestone: | → 2.0 |
---|
If change is trivial and as result something good happens without drawbacks, then perhaps mloskot could just do the change?