CPLSetErrorHandler(): avoid later crashes when passing a null callback.

Fixes #298 (but in a different way that the PR itself) git-svn-id: https://svn.osgeo.org/gdal/trunk@41428 f0d54148-0727-0410-94bb-9a71ac55c965
OSGeo · Feb 7, 2018 · e647e14 · e647e14
1 parent 955b9dd
commit e647e14
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 18 deletions.
diff --git a/autotest/cpp/test_cpl.cpp b/autotest/cpp/test_cpl.cpp
@@ -924,6 +924,15 @@ namespace tut
         CPLPopErrorHandler();
 
         CPLSetConfigOption("CPL_DEBUG", oldVal.size() ? oldVal.c_str() : nullptr);
+
+        oldHandler = CPLSetErrorHandler(nullptr);
+        CPLDebug("TEST", "Test");
+        CPLError(CE_Failure, CPLE_AppDefined, "test");
+        CPLErrorHandler newOldHandler = CPLSetErrorHandler(nullptr);
+        ensure_equals(newOldHandler, static_cast<CPLErrorHandler>(nullptr));
+        CPLDebug("TEST", "Test");
+        CPLError(CE_Failure, CPLE_AppDefined, "test");
+        CPLSetErrorHandler(oldHandler);
     }
 
 /************************************************************************/

diff --git a/gdal/port/cpl_error.cpp b/gdal/port/cpl_error.cpp
@@ -651,10 +651,12 @@ void CPLDebug( const char * pszCategory,
 /* -------------------------------------------------------------------- */
 /*      Invoke the current error handler.                               */
 /* -------------------------------------------------------------------- */
+    bool bDebugProcessed = false;
     if( psCtx->psHandlerStack != nullptr )
     {
         if( psCtx->psHandlerStack->bCatchDebug )
         {
+            bDebugProcessed = true;
             psCtx->psHandlerStack->pfnHandler( CE_Debug, CPLE_None,
                                                pszMessage );
         }
@@ -665,30 +667,28 @@ void CPLDebug( const char * pszCategory,
             {
                 if( psNode->bCatchDebug )
                 {
+                    bDebugProcessed = true;
                     psNode->pfnHandler( CE_Debug, CPLE_None, pszMessage );
                     break;
                 }
                 psNode = psNode->psNext;
             }
-            if( psNode == nullptr )
-            {
-                CPLMutexHolderD( &hErrorMutex );
-                if( gbCatchDebug )
-                    pfnErrorHandler( CE_Debug, CPLE_None, pszMessage );
-                else
-                    CPLDefaultErrorHandler( CE_Debug, CPLE_None, pszMessage );
-            }
         }
     }
-    else
+
+    if( !bDebugProcessed )
     {
         CPLMutexHolderD( &hErrorMutex );
-        if( pfnErrorHandler != nullptr )
+        if( gbCatchDebug )
         {
-            if( gbCatchDebug )
+            if( pfnErrorHandler != nullptr )
+            {
                 pfnErrorHandler( CE_Debug, CPLE_None, pszMessage );
-            else
-                CPLDefaultErrorHandler( CE_Debug, CPLE_None, pszMessage );
+            }
+        }
+        else
+        {
+            CPLDefaultErrorHandler( CE_Debug, CPLE_None, pszMessage );
         }
     }
 
@@ -1091,10 +1091,7 @@ CPLSetErrorHandlerEx( CPLErrorHandler pfnErrorHandlerNew, void* pUserData )
 
         pfnOldHandler = pfnErrorHandler;
 
-        if( pfnErrorHandler == nullptr )
-            pfnErrorHandler = CPLDefaultErrorHandler;
-        else
-            pfnErrorHandler = pfnErrorHandlerNew;
+        pfnErrorHandler = pfnErrorHandlerNew;
 
         pErrorHandlerUserData = pUserData;
     }
@@ -1255,7 +1252,7 @@ void CPL_STDCALL CPLPopErrorHandler()
  * debug messages. In some cases, this might not be desirable and the user
  * would prefer that the previous installed handler (or the default one if no
  * previous installed handler exists in the stack) deal with it. In which
- * case, this function should be called with bCatchDebug.
+ * case, this function should be called with bCatchDebug = FALSE.
  *
  * @param bCatchDebug FALSE if the current error handler should not intercept
  * debug messages