Support a default destructor for PJ objects

5 files changed, 29 insertions, 170 deletions

diff --git a/src/pj_ctx.c b/src/pj_ctx.c
index 89b2816f..32a3d690 100644
--- a/src/pj_ctx.c
+++ b/src/pj_ctx.c
@@ -38,6 +38,8 @@ static volatile int       default_context_initialized = 0;
 projCtx pj_get_ctx( projPJ pj )
 
 {
+    if (0==pj)
+        return pj_get_default_ctx ();
     return pj->ctx;
 }
 
diff --git a/src/pj_init.c b/src/pj_init.c
index 7448fdb6..08a302c6 100644
--- a/src/pj_init.c
+++ b/src/pj_init.c
@@ -769,8 +769,8 @@ bum_call: /* cleanup error return */
 /*      This is the application callable entry point for destroying     */
 /*      a projection definition.  It does work generic to all           */
 /*      projection types, and then calls the projection specific        */
-/*      free function (P->pfree()) to do local work.  This maps to      */
-/*      the FREEUP code in the individual projection source files.      */
+/*      free function (P->pfree()) to do local work.  In most cases     */
+/*      P->pfree()==pj_default_destructor.                              */
 /************************************************************************/
 
 void
@@ -784,15 +784,10 @@ pj_free(PJ *P) {
             pj_dalloc(t);
         }
 
-        /* free array of grid pointers if we have one */
-        if( P->gridlist != NULL )
-            pj_dalloc( P->gridlist );
-
-        if( P->vgridlist_geoid != NULL )
-            pj_dalloc( P->vgridlist_geoid );
-
-        if( P->catalog_name != NULL )
-            pj_dalloc( P->catalog_name );
+        /* free grid lists */
+        pj_dealloc( P->gridlist );
+        pj_dealloc( P->vgridlist_geoid );
+        pj_dealloc( P->catalog_name );
 
         /* We used to call pj_dalloc( P->catalog ), but this will leak */
         /* memory. The safe way to clear catalog and grid is to call */
@@ -800,34 +795,10 @@ pj_free(PJ *P) {
         /* TODO: we should probably have a public pj_cleanup() method to do all */
         /* that */
 
-        if( P->geod != NULL )
-            pj_dalloc( P->geod );
+        /* free the interface to Charles Karney's geodesic library */
+        pj_dealloc( P->geod );
 
         /* free projection parameters */
-        P->pfree(P);
+        P->destructor (P, 0);
     }
 }
-
-
-
-
-
-
-
-
-/************************************************************************/
-/*                              pj_prepare()                            */
-/*                                                                      */
-/*      Helper function for the PJ_xxxx functions providing the         */
-/*      projection specific setup for each projection type.             */
-/*                                                                      */
-/*      Currently not used, but placed here as part of the  material    */
-/*      Demonstrating the idea for a future PJ_xxx architecture         */
-/*      (cf. pj_minimal.c)                                              */
-/*                                                                      */
-/************************************************************************/
-void pj_prepare (PJ *P, const char *description, void (*freeup)(struct PJconsts *), size_t sizeof_struct_opaque) {
-    P->descr = description;
-    P->pfree = freeup;
-    P->opaque  = pj_calloc (1, sizeof_struct_opaque);
-}
diff --git a/src/pj_malloc.c b/src/pj_malloc.c
index 330b14a6..52cf7deb 100644
--- a/src/pj_malloc.c
+++ b/src/pj_malloc.c
@@ -115,7 +115,7 @@ pointer" to signal an error in a multi level allocation:
 
 
 /*****************************************************************************/
-static void *pj_freeup_msg_plain (PJ *P, int errlev) {         /* Destructor */
+void *pj_default_destructor (PJ *P, int errlev) {   /* Destructor */
 /*****************************************************************************
     Does memory deallocation for "plain" PJ objects, i.e. that vast majority
     of PJs where the opaque object does not contain any additionally
@@ -123,54 +123,13 @@ static void *pj_freeup_msg_plain (PJ *P, int errlev) {         /* Destructor */
 ******************************************************************************/
     if (0==P)
         return 0;
-
+    
     if (0!=errlev)
         pj_ctx_set_errno (P->ctx, errlev);
-
+    
     if (0==P->opaque)
         return pj_dealloc (P);
 
     pj_dealloc (P->opaque);
     return pj_dealloc(P);
 }
-
-
-/*****************************************************************************/
-void pj_freeup_plain (PJ *P) {
-/*****************************************************************************
-    Adapts pj_freeup_msg_plain to the format defined for the callback in
-    the PJ object.
-
-    i.e. reduces most instances of projection deallocation code to:
-
-	static void freeup (PJ *P) {
- 	    pj_freeup_plain (P);
- 	    return;
- 	}
-
-	rather than:
-
- 	static void *freeup_msg_add (PJ *P, int errlev) {
- 	    if (0==P)
- 	        return 0;
- 	    pj_ctx_set_errno (P->ctx, errlev);
-
- 	    if (0==P->opaque)
- 	        return pj_dealloc (P);
-
- 	    (* projection specific deallocation goes here *)
-
- 	    pj_dealloc (P->opaque);
- 	    return pj_dealloc(P);
- 	}
-
- 	(* Adapts pipeline_freeup to the format defined for the PJ object *)
- 	static void freeup_msg_add (PJ *P) {
- 	    freeup_new_add (P, 0);
- 	    return;
- 	}
-
- ******************************************************************************/
-    pj_freeup_msg_plain (P, 0);
-    return;
- }
diff --git a/src/pj_obs_api.c b/src/pj_obs_api.c
index 5c9f398b..acf0b577 100644
--- a/src/pj_obs_api.c
+++ b/src/pj_obs_api.c
@@ -12,7 +12,7 @@
  * Author:   Thomas Knudsen,  thokn@sdfe.dk,  2016-06-09/2016-11-06
  *
  ******************************************************************************
- * Copyright (c) 2016, Thomas Knudsen/SDFE
+ * Copyright (c) 2016, 2017 Thomas Knudsen/SDFE 
  *
  * Permission is hereby granted, free of charge, to any person obtaining a
  * copy of this software and associated documentation files (the "Software"),
@@ -36,7 +36,7 @@
 #include <proj.h>
 #include "proj_internal.h"
 #include "projects.h"
-#include <geodesic.h>
+#include "geodesic.h"
 #include <stddef.h>
 #include <errno.h>
 
@@ -407,34 +407,20 @@ PJ *proj_destroy (PJ *P) {
     return 0;
 }
 
-/* For now, if PJ itself is clean, we return the thread local error level. */
-/* This may change as OBS_API error reporting matures */
 int proj_errno (PJ *P) {
-    if (0==P)
-        return pj_ctx_get_errno (pj_get_default_ctx ());
-    if (0 != P->last_errno)
-        return P->last_errno;
     return pj_ctx_get_errno (pj_get_ctx (P));
 }
 
 /*****************************************************************************/
 void proj_errno_set (PJ *P, int err) {
 /******************************************************************************
-    Sets errno in the PJ, and bubbles it up to the context and pj_errno levels
-    through the low level pj_ctx interface.
+    Sets errno at the context and bubble it up to the thread local errno
 ******************************************************************************/
-    if (0==P) {
-        errno = EINVAL;
-        return;
-    }
-
     /* Use proj_errno_reset to explicitly clear the error status */
     if (0==err)
         return;
 
-    /* set local error level */
-    P->last_errno = err;
-    /* and let it bubble up */
+    /* For P==0 err goes to the default context */
     proj_context_errno_set (pj_get_ctx (P), err);
     errno = err;
     return;
@@ -454,14 +440,16 @@ void proj_errno_restore (PJ *P, int err) {
 
     See usage example under proj_errno_reset ()
 ******************************************************************************/
+    if (0==err)
+        return;
     proj_errno_set (P, err);
 }
 
 /*****************************************************************************/
 int proj_errno_reset (PJ *P) {
 /******************************************************************************
-    Clears errno in the PJ, and bubbles it up to the context and
-    pj_errno levels through the low level pj_ctx interface.
+    Clears errno in the context and thread local levels
+    through the low level pj_ctx interface.
 
     Returns the previous value of the errno, for convenient reset/restore
     operations:
@@ -471,24 +459,17 @@ int proj_errno_reset (PJ *P) {
 
         do_something_with_P (P);
 
-        (* failure - keep latest error status *)
+        // failure - keep latest error status
         if (proj_errno(P))
             return;
-        (* success - restore previous error status *)
+        // success - restore previous error status
         proj_errno_restore (P, last_errno);
         return;
     }
 ******************************************************************************/
     int last_errno;
-    if (0==P) {
-        errno = EINVAL;
-        return EINVAL;
-    }
     last_errno = proj_errno (P);
 
-    /* set local error level */
-    P->last_errno = 0;
-    /* and let it bubble up */
     pj_ctx_set_errno (pj_get_ctx (P), 0);
     errno = 0;
     return last_errno;
diff --git a/src/projects.h b/src/projects.h
index 872bf816..ece88cde 100644
--- a/src/projects.h
+++ b/src/projects.h
@@ -253,62 +253,8 @@ struct PJconsts {
 
     void (*spc)(LP, PJ *, struct FACTORS *);
 
-    void (*pfree)(PJ *);
-
-    /*************************************************************************************
-
-                              E R R O R   R E P O R T I N G
-
-    **************************************************************************************
-
-        Currently, we're doing error reporting through the context->last_errno indicator.
-
-        It is, however, not entirely sure this will be the right way to do it in all
-        cases: During allocation/initialization, it is certainly nice to have a higher
-        level error indicator, since we primarily signal "something went wrong", by
-        returning 0 from the pj_init family of functions - and with a null return we
-        cannot pass messages through internal state in the PJ object.
-
-        Historically, the errno variable has been used for that kind of messages, but
-        apparently, thread safety was added to PROJ.4 at a time where it was not clear
-        that errno is actually thread local.
-
-        Additionally, errno semantics has historically been misinterpreted in parts of
-        pj_init.c, a misinterpretation, that was mitigated by a hack in pj_malloc.c
-        some 15 years ago.
-
-        This PJ-local errno is a first step towards a more structured approach to
-        error reporting, being implemented in the OBS_API (cf. pj_obs_api.[ch]), and
-        related plumbing efforts.
-        
-        In due course this will let us get rid of the pj_malloc.c hack, and allow us
-        to introduce a more layered error reporting structure, where errors are
-        reported where they occur, and bubble up to the higher levels (context->errno,
-        then thread local errno), so the highest level indicate "something went wrong
-        somewhere", then the more localized ones can be used for pinpointing: 
-        
-        errno:               "something went wrong somewhere on this thread",
-        context->last_errno: "something went wrong in some PROJ.4 related code",
-        PJ->last_errno:      "It was in THIS PJ something went wrong",
-        pj_strerrno:         "This was what went wrong".
-
-        Which will be quite helpful, once fully implemented, especially for 
-        debugging complex transformation pipelines, while still maintaining backward
-        compatibility in the messaging system.
-
-        Note that there is even a global pj_errno, which is here and there accessed
-        without acquiring lock. This, and the practise of resetting the thread local
-        errno, should be given some consideration during the cleanup of the error
-        reporting system.
-
-        The name "last_errno", rather than "errno" is used partially for alignment
-        with the context->last_errno, partially because in a multithreaded environment,
-        errno is a macro, and spurious spaces turning "errno" into a separate token
-        will expose it to macro expansion, to the tune of much confusion and agony.
-
-    **************************************************************************************/
-    int last_errno;
-
+    void *(*destructor)(PJ *, int);
+    
 
     /*************************************************************************************
 
@@ -616,7 +562,7 @@ C_NAMESPACE PJ *pj_##name (PJ *P) {                          \
     P = (PJ*) pj_calloc (1, sizeof(PJ));                     \
     if (0==P)                                                \
         return 0;                                            \
-    P->pfree = freeup;                                       \
+    P->destructor = pj_default_destructor;                   \
     P->descr = des_##name;                                   \
     return P;                                                \
 }                                                            \
@@ -707,8 +653,6 @@ int pj_datum_set(projCtx,paralist *, PJ *);
 int pj_prime_meridian_set(paralist *, PJ *);
 int pj_angular_units_set(paralist *, PJ *);
 
-void pj_prepare (PJ *P, const char *description, void (*freeup)(struct PJconsts *), size_t sizeof_struct_opaque);
-
 paralist *pj_clone_paralist( const paralist* );
 paralist *pj_search_initcache( const char *filekey );
 void      pj_insert_initcache( const char *filekey, const paralist *list);
@@ -822,6 +766,8 @@ struct PJ_LIST             *pj_get_list_ref( void );
 struct PJ_SELFTEST_LIST    *pj_get_selftest_list_ref ( void );
 struct PJ_PRIME_MERIDIANS  *pj_get_prime_meridians_ref( void );
 
+void *pj_default_destructor (PJ *P, int errlev);
+
 double pj_atof( const char* nptr );
 double pj_strtod( const char *nptr, char **endptr );
 void   pj_freeup_plain (PJ *P);