GDALDestroyWarpOptions to do nothing for null

[Mateusz Łoskot]

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).

[Jukka Rahkonen]

If change is trivial and as result something good happens without drawbacks, then perhaps mloskot could just do the change?

[Mateusz Łoskot]

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.

[Even Rouault]

Mateusz, looks go to me. Please go ahead with the change

[Mateusz Łoskot]

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

1. Do we want to replace VALIDATE_POINTER0 with if nullptr do no-op strategy? I think it would be good to unify all GDALDestroy*, as are those at the bottom of the list above.

2. The two with no-check, GDALDestroyGeoLocTransformer and GDALDestroyRPCTransformer, should be updated to either do VALIDATE_POINTER0 check or do no-op for nullptr input. Which one to go for?

3. What to do with GDALDestroyTransformer?

[Even Rouault]

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.

[Mateusz Łoskot]

Finally, discussed changes applied in trunk @ r29207

[Even Rouault]

Milestone renamed