dreid 01/05/13 05:01:29
Modified: . configure.in
include apr_memory_system.h
memory/unix apr_memory_system.c
Log:
OK, this commit basically tries to address the assert vs check thing we've all
been talking about. Essentially it's my opinion that in a production build we
shouldn't be using asserts, but they do have a use for developers and can
really
help with reducing the time it takes to find and fix a problem. So, this adds
a new configure option, --enable-assert-memory which adds a define to the
build
that includes asserts. My criteria for having an assert is that it has to
provide
additional information over any checks that already exist. for instance,
look at
apr_sms_malloc where the asserts are there as otherwise we miss out on why we
return NULL. I've tried to follow this but this is just the start.
I've added an extra error message that we return when we have the opportunity
and an invalid mem_sys is passed in, APR_EMEMSYS.
Comments?
Revision Changes Path
1.306 +5 -1 apr/configure.in
Index: configure.in
===================================================================
RCS file: /home/cvs/apr/configure.in,v
retrieving revision 1.305
retrieving revision 1.306
diff -u -r1.305 -r1.306
--- configure.in 2001/05/13 11:13:14 1.305
+++ configure.in 2001/05/13 12:01:27 1.306
@@ -141,11 +141,15 @@
])dnl
AC_ARG_ENABLE(maintainer-mode,[ --enable-maintainer-mode Turn on debugging
and compile time warnings],
- [APR_ADDTO(CFLAGS,-g)
+ [APR_ADDTO(CFLAGS,-g -DAPR_ASSERT_MEMORY)
if test "$GCC" = "yes"; then
APR_ADDTO(CFLAGS,[-Wall -Wmissing-prototypes -Wstrict-prototypes
-Wmissing-declarations])
fi
])dnl
+
+AC_ARG_ENABLE(assert-memory,[ --enable-assert-memory Turn on asserts in the
APR memory code],
+ [APR_ADDTO(CFLAGS,-DAPR_ASSERT_MEMORY)
+])
dnl # this is the place to put specific options for platform/compiler
dnl # combinations
1.6 +3 -4 apr/include/apr_memory_system.h
Index: apr_memory_system.h
===================================================================
RCS file: /home/cvs/apr/include/apr_memory_system.h,v
retrieving revision 1.5
retrieving revision 1.6
diff -u -r1.5 -r1.6
--- apr_memory_system.h 2001/05/13 11:11:42 1.5
+++ apr_memory_system.h 2001/05/13 12:01:28 1.6
@@ -176,15 +176,14 @@
* memory system from your apr_xxx_sms_create.
* @deffunc void apr_sms_validate(apr_sms_t *mem_sys)
*/
-#ifdef APR_MEMORY_SYSTEM_DEBUG
-APR_DECLARE(void)
-apr_sms_assert(apr_sms_t *mem_sys);
+#ifdef APR_MEMORY_ASSERT
+APR_DECLARE(void) apr_sms_assert(apr_sms_t *mem_sys);
#else
#ifdef apr_sms_assert
#undef apr_sms_assert
#endif
#define apr_sms_assert(mem_sys)
-#endif /* APR_MEMORY_SYSTEM_DEBUG */
+#endif /* APR_MEMORY_ASSERT */
/**
* Reset a memory system so it can be reused.
1.5 +66 -25 apr/memory/unix/apr_memory_system.c
Index: apr_memory_system.c
===================================================================
RCS file: /home/cvs/apr/memory/unix/apr_memory_system.c,v
retrieving revision 1.4
retrieving revision 1.5
diff -u -r1.4 -r1.5
--- apr_memory_system.c 2001/05/13 11:11:45 1.4
+++ apr_memory_system.c 2001/05/13 12:01:29 1.5
@@ -87,9 +87,14 @@
APR_DECLARE(void *) apr_sms_malloc(apr_sms_t *mem_sys,
apr_size_t size)
{
- assert(mem_sys != NULL);
- assert(mem_sys->malloc_fn != NULL);
-
+#ifdef APR_ASSERT_MEMORY
+ assert(mem_sys);
+ assert(mem_sys->malloc_fn);
+#endif
+
+ if (!mem_sys || !mem_sys->malloc_fn)
+ return NULL;
+
if (size == 0)
return NULL;
@@ -99,7 +104,11 @@
APR_DECLARE(void *) apr_sms_calloc(apr_sms_t *mem_sys,
apr_size_t size)
{
- assert(mem_sys != NULL);
+#ifdef APR_ASSERT_MEMORY
+ assert(mem_sys);
+ assert(mem_sys->malloc_fn);
+ assert(mem_sys->calloc_fn);
+#endif
if (size == 0)
return NULL;
@@ -118,9 +127,11 @@
APR_DECLARE(void *) apr_sms_realloc(apr_sms_t *mem_sys, void *mem,
apr_size_t size)
{
- assert(mem_sys != NULL);
- assert(mem_sys->realloc_fn != NULL);
-
+#ifdef APR_ASSERT_MEMORY
+ assert(mem_sys);
+ assert(mem_sys->realloc_fn);
+#endif
+
if (mem == NULL)
return apr_sms_malloc(mem_sys, size);
@@ -136,7 +147,12 @@
APR_DECLARE(apr_status_t) apr_sms_free(apr_sms_t *mem_sys,
void *mem)
{
- assert(mem_sys != NULL);
+#ifdef APR_ASSERT_MEMORY
+ assert(mem_sys->free_fn);
+#endif
+
+ if (!mem_sys)
+ return APR_EMEMSYS;
if (mem == NULL)
return APR_EINVAL;
@@ -153,6 +169,9 @@
static int apr_sms_is_tracking(apr_sms_t *mem_sys)
{
+#ifdef APR_ASSERT_MEMORY
+ assert(mem_sys->reset_fn);
+#endif
/*
* The presense of a reset function gives us the clue that this is a
* tracking memory system.
@@ -199,7 +218,7 @@
return mem_sys;
}
-#ifdef APR_MEMORY_SYSTEM_DEBUG
+#ifdef APR_MEMORY_ASSERT
APR_DECLARE(void) apr_sms_assert(apr_sms_t *mem_sys)
{
apr_sms_t *parent;
@@ -257,7 +276,7 @@
* parent.
*/
}
-#endif /* APR_MEMORY_SYSTEM_DEBUG */
+#endif /* APR_MEMORY_ASSERT */
/*
* LOCAL FUNCTION used in:
@@ -306,9 +325,10 @@
APR_DECLARE(apr_status_t) apr_sms_reset(apr_sms_t *mem_sys)
{
- assert(mem_sys != NULL);
- /* Assert when called on a non-tracking memory system */
- assert(mem_sys->reset_fn != NULL);
+ if (!mem_sys)
+ return APR_EMEMSYS;
+ if (!mem_sys->reset_fn)
+ return APR_EMEMALLOCATOR;
/*
* Run the cleanups of all child memory systems _including_
@@ -342,7 +362,8 @@
struct apr_sms_cleanup *cleanup;
struct apr_sms_cleanup *next_cleanup;
- assert(mem_sys != NULL);
+ if (!mem_sys)
+ return APR_EMEMSYS;
if (apr_sms_is_tracking(mem_sys))
{
@@ -457,7 +478,7 @@
apr_sms_free(mem_sys->parent_mem_sys, mem_sys);
/* 4 - Assume we are the child of a tracking memory system, and do nothing
*/
-#ifdef APR_MEMORY_SYSTEM_DEBUG
+#ifdef APR_ASSERT_MEMORY
mem_sys = mem_sys->parent_mem_sys;
while (mem_sys)
{
@@ -466,17 +487,21 @@
mem_sys = mem_sys->parent_mem_sys;
}
-
assert(0); /* Made the wrong assumption, so we assert */
-#endif /* APR_MEMORY_SYSTEM_DEBUG */
+#endif /* APR_MEMORY_ASSERT */
return APR_SUCCESS;
}
APR_DECLARE(apr_status_t) apr_sms_is_ancestor(apr_sms_t *a,
apr_sms_t *b)
{
+#ifdef APR_ASSERT_MEMORY
+ assert(a != NULL);
assert(b != NULL);
-
+#endif
+ if (!b)
+ return APR_EINVAL;
+
while (b && b != a)
b = b->parent_mem_sys;
@@ -484,18 +509,26 @@
return !(b == a);
}
-APR_DECLARE(void)
-apr_sms_threadsafe_lock(apr_sms_t *mem_sys)
+APR_DECLARE(void) apr_sms_threadsafe_lock(apr_sms_t *mem_sys)
{
+#ifdef APR_ASSERT_MEMORY
assert(mem_sys != NULL);
+#endif
+
+ if (!mem_sys)
+ return;
if (mem_sys->threadsafe_lock_fn != NULL)
mem_sys->threadsafe_lock_fn(mem_sys);
}
-APR_DECLARE(void)
-apr_sms_threadsafe_unlock(apr_sms_t *mem_sys)
+APR_DECLARE(void) apr_sms_threadsafe_unlock(apr_sms_t *mem_sys)
{
+#ifdef APR_ASSERT_MEMORY
assert(mem_sys != NULL);
+#endif
+
+ if (!mem_sys)
+ return;
if (mem_sys->threadsafe_unlock_fn != NULL)
mem_sys->threadsafe_unlock_fn(mem_sys);
}
@@ -510,15 +543,19 @@
{
struct apr_sms_cleanup *cleanup;
- assert(mem_sys != NULL);
+#ifdef APR_ASSERT_MEMORY
assert(mem_sys->accounting_mem_sys != NULL);
+#endif
+ if (!mem_sys)
+ return APR_EMEMSYS;
+
if (cleanup_fn == NULL)
return APR_EINVAL;
cleanup = (struct apr_sms_cleanup *)
apr_sms_malloc(mem_sys->accounting_mem_sys,
- sizeof(struct apr_sms_cleanup));
+ sizeof(struct apr_sms_cleanup));
if (cleanup == NULL)
return APR_ENOMEM;
@@ -538,9 +575,13 @@
struct apr_sms_cleanup *cleanup;
struct apr_sms_cleanup **cleanup_ref;
- assert(mem_sys != NULL);
+#ifdef APR_ASSERT_MEMORY
assert(mem_sys->accounting_mem_sys != NULL);
+#endif
+ if (!mem_sys)
+ return APR_EMEMSYS;
+
cleanup = mem_sys->cleanups;
cleanup_ref = &mem_sys->cleanups;
while (cleanup)
