This changes apr_sms_create into a more appropriately named

apr_sms_init. It also now returns an apr_status_t which is more
inline with the rest of the code.

I've changed to using calloc for the memory structures rather
than memset in the function, but we may want to change this again.
This patch is a modified version of one submitted by Sander Striker.

Submitted by:	Sander Striker <striker@samba-tng.org>
Reviewed by:	David Reid (applied in a modified form)


git-svn-id: http://svn.apache.org/repos/asf/apr/apr/trunk@61689 13f79535-47bb-0310-9956-ffa450edef68

4 files changed, 45 insertions, 32 deletions

diff --git a/include/apr_sms.h b/include/apr_sms.h
index e29db6ba7..d616264f3 100644
--- a/include/apr_sms.h
+++ b/include/apr_sms.h
@@ -161,20 +161,18 @@ APR_DECLARE(apr_status_t) apr_sms_free(apr_sms_t *mem_sys, void *mem);
  */
 
 /**
- * Create a memory system (actually it initialized a memory system structure)
+ * Initialize a memory system
  * @caution Call this function as soon as you have obtained a block of memory
  *          to serve as a memory system structure from your 
  *          apr_xxx_sms_create. Only use this function when you are
  *          implementing a memory system.
- * @param memory The memory to turn into a memory system
- * @warning The memory passed in should be at least of size 
- *          sizeof(apr_sms_t)
+ * @param mem_sys The memory system created
  * @param parent_mem_sys The parent memory system
- * @return The freshly initialized memory system
- * @deffunc apr_sms_t *apr_sms_create(void *memory,
- *				   apr_sms_t *parent_mem_sys)
+ * @deffunc apr_status_t apr_sms_init(apr_sms_t *sms,
+ *				      apr_sms_t *parent_mem_sys)
  */
-APR_DECLARE(apr_sms_t *) apr_sms_create(void *memory, apr_sms_t *parent_mem_sys);
+APR_DECLARE(apr_status_t) apr_sms_init(apr_sms_t *mem_sys, 
+                                       apr_sms_t *parent_mem_sys);
 
 /**
  * Check if a memory system is obeying all rules. 
diff --git a/memory/unix/apr_sms.c b/memory/unix/apr_sms.c
index 9f0bc1136..202ccfb43 100644
--- a/memory/unix/apr_sms.c
+++ b/memory/unix/apr_sms.c
@@ -177,37 +177,40 @@ static int apr_sms_is_tracking(apr_sms_t *mem_sys)
     return mem_sys->reset_fn != NULL;
 }
 
-APR_DECLARE(apr_sms_t *) apr_sms_create(void *memory, 
-                                        apr_sms_t *parent_mem_sys)
+APR_DECLARE(apr_status_t) apr_sms_init(apr_sms_t *mem_sys, 
+                                       apr_sms_t *parent_mem_sys)
 {
-    apr_sms_t *mem_sys;
-
-    if (!memory)
-        return NULL;
+    /* XXX - I've assumed that memory passed in will be zeroed,
+     * i.e. calloc'd instead of malloc'd...
+     * This may well be a bogus assumption, and if so we either need
+     * to memset or have a series of =NULL's at the end.
+     * This function is only called by modules, so this isn't as crazy
+     * an assumption to make as it sounds :)
+     */
 
-    /* Just typecast it, and clear it */
-    mem_sys = (apr_sms_t *)memory;
-    memset(mem_sys, '\0', sizeof(apr_sms_t));
+    if (!mem_sys)
+        return APR_EINVAL;
 
-    /* Initialize the parent and accounting memory system pointers */
     mem_sys->parent_mem_sys = parent_mem_sys;
     mem_sys->accounting_mem_sys = mem_sys;
+    mem_sys->child_mem_sys = NULL;
 
-    if (parent_mem_sys != NULL) {
+    if (parent_mem_sys) {
         if ((mem_sys->sibling_mem_sys = parent_mem_sys->child_mem_sys) != NULL)
             mem_sys->sibling_mem_sys->ref_mem_sys = &mem_sys->sibling_mem_sys;
 
         mem_sys->ref_mem_sys = &parent_mem_sys->child_mem_sys;
+        /* This is probably not correct as we could have multiple children
+         * from a single parent...  We probably need a list...
+         */
         parent_mem_sys->child_mem_sys = mem_sys;
     }
-    /* This seems a bit redundant, but we're not taking chances */
     else {
         mem_sys->ref_mem_sys     = NULL;
         mem_sys->sibling_mem_sys = NULL;
-        mem_sys->child_mem_sys   = NULL;
     }
 
-    return mem_sys;
+    return APR_SUCCESS;
 }
 
 #ifdef APR_MEMORY_ASSERT
diff --git a/memory/unix/apr_sms_std.c b/memory/unix/apr_sms_std.c
index bdf54d7ee..328615c66 100644
--- a/memory/unix/apr_sms_std.c
+++ b/memory/unix/apr_sms_std.c
@@ -108,22 +108,28 @@ static apr_status_t apr_sms_std_free(apr_sms_t *mem_sys,
 APR_DECLARE(apr_status_t) apr_sms_std_create(apr_sms_t **mem_sys)
 {
     apr_sms_t *new_mem_sys;
+    apr_status_t rv;
 
     assert(mem_sys);
 
     *mem_sys = NULL;
-    /* should we be using apr_sms_calloc now we have it??? */
-    new_mem_sys = apr_sms_create(malloc(sizeof(apr_sms_t)),
-                                 NULL);
+    /* We don't have a parent so we allocate the memory
+     * for the structure ourselves...
+     */
+    new_mem_sys = apr_sms_std_calloc(NULL, sizeof(apr_sms_t));
 
     if (!new_mem_sys)
         return APR_ENOMEM;
 
+    if ((rv = apr_sms_init(new_mem_sys, NULL)) != APR_SUCCESS)
+        return rv;
+
     new_mem_sys->malloc_fn  = apr_sms_std_malloc;
     new_mem_sys->calloc_fn  = apr_sms_std_calloc;
     new_mem_sys->realloc_fn = apr_sms_std_realloc;
     new_mem_sys->free_fn    = apr_sms_std_free;
     new_mem_sys->identity   = module_identity;
+
     /* as we're not a tracking memory module, i.e. we don't keep
      * track of our allocations, we don't have apr_sms_reset or
      * apr_sms_destroy functions.
@@ -134,3 +140,4 @@ APR_DECLARE(apr_status_t) apr_sms_std_create(apr_sms_t **mem_sys)
     *mem_sys = new_mem_sys;
     return APR_SUCCESS;
 }
+
diff --git a/memory/unix/apr_sms_tracking.c b/memory/unix/apr_sms_tracking.c
index 40f48d2c7..0fda45b5e 100644
--- a/memory/unix/apr_sms_tracking.c
+++ b/memory/unix/apr_sms_tracking.c
@@ -173,7 +173,7 @@ static apr_status_t apr_sms_tracking_free(apr_sms_t *mem_sys,
                                           void *mem)
 {
     apr_track_node_t *node;
-
+    
     assert(mem_sys);
     assert(mem);
 
@@ -192,7 +192,7 @@ static apr_status_t apr_sms_tracking_reset(apr_sms_t *mem_sys)
     apr_sms_tracking_t *tms;
     apr_track_node_t *node;
     apr_status_t rv;
-    
+ 
     assert(mem_sys);
 
     tms = (apr_sms_tracking_t *)mem_sys;
@@ -233,22 +233,26 @@ static apr_status_t apr_sms_tracking_destroy(apr_sms_t *mem_sys)
 APR_DECLARE(apr_status_t) apr_sms_tracking_create(apr_sms_t **mem_sys, 
                                                   apr_sms_t *pms)
 {
-    apr_sms_t *new_mem_sys, *tmpms;
+    apr_sms_t *new_mem_sys;
     apr_sms_tracking_t *tms;
+    apr_status_t rv;
 
     assert(mem_sys);
     assert(pms);
     
     *mem_sys = NULL;
-    /* changed this to 2 stages to make easier to follow...
-     * should we be using apr_sms_calloc now we have it? 
+    /* We're not a top level module, ie we have a parent, so
+     * we allocate the memory for the structure from our parent.
+     * This is safe as we shouldn't outlive our parent...
      */
-    tmpms = apr_sms_malloc(pms, sizeof(apr_sms_tracking_t));
-    new_mem_sys = apr_sms_create(tmpms, pms);
+    new_mem_sys = apr_sms_calloc(pms, sizeof(apr_sms_tracking_t));
 
     if (!new_mem_sys)
         return APR_ENOMEM;
 
+    if ((rv = apr_sms_init(new_mem_sys, pms)) != APR_SUCCESS)
+        return rv;
+
     new_mem_sys->malloc_fn  = apr_sms_tracking_malloc;
     new_mem_sys->calloc_fn  = apr_sms_tracking_calloc;
     new_mem_sys->realloc_fn = apr_sms_tracking_realloc;
@@ -265,3 +269,4 @@ APR_DECLARE(apr_status_t) apr_sms_tracking_create(apr_sms_t **mem_sys,
     *mem_sys = new_mem_sys;
     return APR_SUCCESS;
 }
+