From fa318ed5db0e00bf0a9fb3c18852efb44a095427 Mon Sep 17 00:00:00 2001 From: Thomas Knudsen Date: Wed, 27 Sep 2017 13:54:30 +0200 Subject: Support a default destructor for PJ objects --- src/pj_malloc.c | 47 +++-------------------------------------------- 1 file changed, 3 insertions(+), 44 deletions(-) (limited to 'src/pj_malloc.c') 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; - } -- cgit v1.2.3 From 664577ced6a8e4074b1f53af82b5ae5d1d189eac Mon Sep 17 00:00:00 2001 From: Thomas Knudsen Date: Wed, 27 Sep 2017 13:56:34 +0200 Subject: Enable default destructor for all PJ objects. In most cases memory deallocation is completely removed from the code since it can be handled by the default destructor. In a few special cases a local destructor overrides the default destructor and makes sure that locally allocated memored is cleaned up correctly. Move all deallocation from pj_free to pj_default_destructor Rename pj_latlong.c to fit with the conventional format PJ_latlong.c - freeup was missed here due to wrong naming Clean up pj_init to avoid double deallocation; Also resolve #576 by adding z_0 and t_0 options in pj_init, while cleaning Add a prototype for dealloc_params Added missing errno.h include in pj_ctx.c Temporarily removing ob_tran from testvarious, to be sure that is where the trouble is Make PJ_ob_tran.c use proper initialization for the chained projection proj=ob_tran: make it clear, that we disallow ellipsoidal projections, and, for improved backwards compatibility, turns off default settings, which could inject unwanted ellipsoid definitions ... then also remove the ellipsoid definition from the testvarious test case - which is probably buggy anyway Work around cs2cs spherical init bug in testvarious; Forbid defs for ob_tran in pj_init --- src/pj_malloc.c | 91 ++++++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 71 insertions(+), 20 deletions(-) (limited to 'src/pj_malloc.c') diff --git a/src/pj_malloc.c b/src/pj_malloc.c index 52cf7deb..9c69257f 100644 --- a/src/pj_malloc.c +++ b/src/pj_malloc.c @@ -40,7 +40,7 @@ ** projection system memory allocation/deallocation call with custom ** application procedures. */ -#include +#include "projects.h" #include /**********************************************************************/ @@ -52,30 +52,43 @@ https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=86420. It seems, that pj_init and similar functions incorrectly (under debian/glibs-2.3.2) assume that pj_malloc resets errno after success. pj_malloc tries to mimic this. -***********************************************************************/ - int old_errno = errno; - void *res = malloc(size); - if ( res && !old_errno ) - errno = 0; - return res; -} -/**********************************************************************/ -void pj_dalloc(void *ptr) { -/**********************************************************************/ - free(ptr); +NOTE (2017-09-29): The problem described at the bugzilla page +referred to above, is most likely a case of someone not +understanding the proper usage of errno. We should review +whether "the problem is actually a problem" in PROJ.4 code. + +Library specific allocators can be useful, and improve +interoperability, if properly used. That is, by making them +run/initialization time switchable, somewhat like the file i/o +interface. + +But as things stand, we are more likely to get benefit +from reviewing the code for proper errno usage, which is hard, +due to the presence of context local and global pj_errnos. + +Probably, these were introduced in order to support incomplete +implementations of thread local errnos at an early phase of the +implementation of multithreading support in PROJ.4). + +It is likely too late to get rid of contexts, but we can still +benefit from a better usage of errno. +***********************************************************************/ + int old_errno = errno; + void *res = malloc(size); + if ( res && !old_errno ) + errno = 0; + return res; } /**********************************************************************/ void *pj_calloc (size_t n, size_t size) { /*********************************************************************** - pj_calloc is the pj-equivalent of calloc(). It allocates space for an array of elements of size . The array is initialized to zeros. - ***********************************************************************/ void *res = pj_malloc (n*size); if (0==res) @@ -85,10 +98,16 @@ The array is initialized to zeros. } +/**********************************************************************/ +void pj_dalloc(void *ptr) { +/**********************************************************************/ + free(ptr); +} + + /**********************************************************************/ void *pj_dealloc (void *ptr) { /*********************************************************************** - pj_dealloc supports the common use case of "clean up and return a null pointer" to signal an error in a multi level allocation: @@ -113,23 +132,55 @@ pointer" to signal an error in a multi level allocation: +/*****************************************************************************/ +void *pj_dealloc_params (projCtx ctx, paralist *start, int errlev) { +/***************************************************************************** + Companion to pj_default_destructor (below). Deallocates a linked list + of "+proj=xxx" initialization parameters. + + Also called from pj_init_ctx when encountering errors before the PJ + proper is allocated. +******************************************************************************/ + paralist *t, *n; + for (t = start; t; t = n) { + n = t->next; + pj_dealloc(t); + } + pj_ctx_set_errno (ctx, errlev); + return (void *) 0; +} + /*****************************************************************************/ 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 - allocated memory. + allocated memory below the P->opaque level. ******************************************************************************/ if (0==P) return 0; - if (0!=errlev) - pj_ctx_set_errno (P->ctx, errlev); + + /* 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 */ + /* pj_gc_unloadall(pj_get_default_ctx()); and pj_deallocate_grids(); */ + /* TODO: we should probably have a public pj_cleanup() method to do all */ + /* that */ + + /* free the interface to Charles Karney's geodesic library */ + pj_dealloc( P->geod ); - if (0==P->opaque) - return pj_dealloc (P); + /* free parameter list elements */ + pj_dealloc_params (pj_get_ctx(P), P->params, errlev); pj_dealloc (P->opaque); + if (0!=errlev) + pj_ctx_set_errno (pj_get_ctx(P), errlev); return pj_dealloc(P); } -- cgit v1.2.3 From ca84e57463cacaa0d6b8f81b11ca6714c77e88c5 Mon Sep 17 00:00:00 2001 From: Thomas Knudsen Date: Tue, 3 Oct 2017 13:47:11 +0200 Subject: Enable address sanitizer in linux/clang build Elim some leaks by initializing PJ.destructor in PJ_ob_tran.c properly Avoid tests bombing when built with address sanitizer: Repair memory leak in test228.c Avoid tests bombing when built with address sanitizer: Repair memory leak in multistresstest.c --- src/pj_malloc.c | 1 - 1 file changed, 1 deletion(-) (limited to 'src/pj_malloc.c') diff --git a/src/pj_malloc.c b/src/pj_malloc.c index 9c69257f..75d05378 100644 --- a/src/pj_malloc.c +++ b/src/pj_malloc.c @@ -161,7 +161,6 @@ void *pj_default_destructor (PJ *P, int errlev) { /* Destructor */ if (0==P) return 0; - /* free grid lists */ pj_dealloc( P->gridlist ); pj_dealloc( P->vgridlist_geoid ); -- cgit v1.2.3 From cb335699aafc84952d1a6a94eb9d2ee201ad416f Mon Sep 17 00:00:00 2001 From: Thomas Knudsen Date: Sun, 8 Oct 2017 16:29:16 +0200 Subject: Resolve #594 and OSS-Fuzz-3569 (#595) * Resolve #594 and OSS-Fuzz-3569 * Restructure PJ_geos opaque object: sweep_axis showed unneeded, and freeing it was wrong. Eliminate instead Resolves #594 Resolves OSS-Fuzz Issue 3569 Credit to OSS-Fuzz --- src/pj_malloc.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'src/pj_malloc.c') diff --git a/src/pj_malloc.c b/src/pj_malloc.c index 75d05378..4e465c46 100644 --- a/src/pj_malloc.c +++ b/src/pj_malloc.c @@ -158,6 +158,13 @@ void *pj_default_destructor (PJ *P, int errlev) { /* Destructor */ of PJs where the opaque object does not contain any additionally allocated memory below the P->opaque level. ******************************************************************************/ + + /* Even if P==0, we set the errlev on pj_error and the default context */ + /* Note that both, in the multithreaded case, may then contain undefined */ + /* values. This is expected behaviour. For MT have one ctx per thread */ + if (0!=errlev) + pj_ctx_set_errno (pj_get_ctx(P), errlev); + if (0==P) return 0; @@ -179,7 +186,5 @@ void *pj_default_destructor (PJ *P, int errlev) { /* Destructor */ pj_dealloc_params (pj_get_ctx(P), P->params, errlev); pj_dealloc (P->opaque); - if (0!=errlev) - pj_ctx_set_errno (pj_get_ctx(P), errlev); return pj_dealloc(P); } -- cgit v1.2.3