You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by jw...@apache.org on 2001/06/05 01:09:38 UTC

cvs commit: apr/misc/unix errorcodes.c

jwoolley    01/06/04 16:09:38

  Modified:    include  apr_errno.h apr_sms.h
               memory/unix apr_sms.c apr_sms_std.c apr_sms_tracking.c
               misc/unix errorcodes.c
  Log:
  * Remove the unnecessary parameter checks and the extra error codes that
    went along with them.  The APR policy is to segfault on a NULL parameter
    rather than silently returning some error code that the caller might
    not check anyway.
  * Also remove lots of unnecessary assertions (where the code would have
    segfaulted anyway, even without an explicit assert).  I've tried to be
    sure that every one I removed will result in a virtually immediate
    segfault anyway.  Ones that don't are the ones that are tricky to debug.
    If I've removed too many, say so and I'll put them back.
  * Fix a misnamed APR_MEMORY_ASSERT -> APR_ASSERT_MEMORY, which was causing
    apr_assert_memory() never to be compiled.  Also fix a syntax error in that
    function that's been there since rev 1.1 of apr_sms.c, which no one's
    ever noticed because they never compiled it before.
  
  Revision  Changes    Path
  1.67      +0 -9      apr/include/apr_errno.h
  
  Index: apr_errno.h
  ===================================================================
  RCS file: /home/cvs/apr/include/apr_errno.h,v
  retrieving revision 1.66
  retrieving revision 1.67
  diff -u -d -u -r1.66 -r1.67
  --- apr_errno.h	2001/05/22 15:08:25	1.66
  +++ apr_errno.h	2001/06/04 23:09:31	1.67
  @@ -180,11 +180,6 @@
    * APR_EGENERAL     General failure (specific information not available)
    * APR_EBADIP       The specified IP address is invalid
    * APR_EBADMASK     The specified netmask is invalid
  - * APR_ENOCLEANUP   There is no memory cleanup available
  - * APR_EMEMSYS      An invalid memory system was passed in to an 
  - *                  apr_sms function
  - * APR_EMEMFUNC     A function was called that isn't available in the
  - *                  selected memory system
    * </PRE>
    *
    * <PRE>
  @@ -253,8 +248,6 @@
   #define APR_EINCOMPLETE    (APR_OS_START_ERROR + 22)
   #define APR_EABOVEROOT     (APR_OS_START_ERROR + 23)
   #define APR_EBADPATH       (APR_OS_START_ERROR + 24)
  -#define APR_ENOCLEANUP     (APR_OS_START_ERROR + 25)
  -#define APR_EMEMSYS        (APR_OS_START_ERROR + 26)
   
   /* APR ERROR VALUE TESTS */
   #define APR_STATUS_IS_ENOSTAT(s)        ((s) == APR_ENOSTAT)
  @@ -281,8 +274,6 @@
   #define APR_STATUS_IS_EINCOMPLETE(s)    ((s) == APR_EINCOMPLETE)
   #define APR_STATUS_IS_EABOVEROOT(s)     ((s) == APR_EABOVEROOT)
   #define APR_STATUS_IS_EBADPATH(s)       ((s) == APR_EBADPATH)
  -#define APR_STATUS_IS_ENOCLEANUP(s)     ((s) == APR_ENOCLEANUP)
  -#define APR_STATUS_IS_EMEMSYS(s)        ((s) == APR_EMEMSYS)
   
   /* APR STATUS VALUES */
   #define APR_INCHILD        (APR_OS_START_STATUS + 1)
  
  
  
  1.8       +2 -2      apr/include/apr_sms.h
  
  Index: apr_sms.h
  ===================================================================
  RCS file: /home/cvs/apr/include/apr_sms.h,v
  retrieving revision 1.7
  retrieving revision 1.8
  diff -u -d -u -r1.7 -r1.8
  --- apr_sms.h	2001/06/01 17:32:12	1.7
  +++ apr_sms.h	2001/06/04 23:09:32	1.8
  @@ -180,14 +180,14 @@
    *          memory system from your apr_xxx_sms_create.
    * @deffunc void apr_sms_validate(apr_sms_t *mem_sys)
    */
  -#ifdef APR_MEMORY_ASSERT
  +#ifdef APR_ASSERT_MEMORY
   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_ASSERT */
  +#endif /* APR_ASSERT_MEMORY */
   
   /**
    * Reset a memory system so it can be reused. 
  
  
  
  1.10      +15 -98    apr/memory/unix/apr_sms.c
  
  Index: apr_sms.c
  ===================================================================
  RCS file: /home/cvs/apr/memory/unix/apr_sms.c,v
  retrieving revision 1.9
  retrieving revision 1.10
  diff -u -d -u -r1.9 -r1.10
  --- apr_sms.c	2001/06/01 17:32:13	1.9
  +++ apr_sms.c	2001/06/04 23:09:33	1.10
  @@ -64,7 +64,10 @@
   #include "apr_general.h"
   #include "apr_sms.h"
   #include <stdlib.h>
  +
  +#ifdef APR_ASSERT_MEMORY
   #include <assert.h>
  +#endif
   
   #include <memory.h> /* strikerXXX: had to add this for windows to stop 
                        * complaining, please autoconf the include stuff
  @@ -88,14 +91,6 @@
   APR_DECLARE(void *) apr_sms_malloc(apr_sms_t *mem_sys,
                                      apr_size_t size)
   {
  -#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;
   
  @@ -105,11 +100,6 @@
   APR_DECLARE(void *) apr_sms_calloc(apr_sms_t *mem_sys,
                                      apr_size_t size)
   {
  -#ifdef APR_ASSERT_MEMORY
  -    assert(mem_sys);
  -    assert(mem_sys->malloc_fn);
  -#endif
  -
       if (size == 0)
           return NULL;
   
  @@ -129,11 +119,6 @@
   APR_DECLARE(void *) apr_sms_realloc(apr_sms_t *mem_sys, void *mem,
                                       apr_size_t size)
   {
  -#ifdef APR_ASSERT_MEMORY
  -    assert(mem_sys);
  -    assert(mem_sys->realloc_fn);
  -#endif
  -   
       if (!mem)
           return apr_sms_malloc(mem_sys, size);
   
  @@ -148,16 +133,6 @@
   APR_DECLARE(apr_status_t) apr_sms_free(apr_sms_t *mem_sys,
                                          void *mem)
   {
  -    if (!mem_sys)
  -        return APR_EMEMSYS;
  -       
  -#ifdef APR_ASSERT_MEMORY
  -    assert(mem_sys->free_fn);
  -#endif
  -
  -    if (!mem)
  -        return APR_EINVAL;
  -
       if (mem_sys->free_fn)
           return mem_sys->free_fn(mem_sys, mem);  
   
  @@ -188,9 +163,6 @@
        * an assumption to make as it sounds :)
        */
   
  -    if (!mem_sys)
  -        return APR_EINVAL;
  -
       mem_sys->parent_mem_sys = parent_mem_sys;
       mem_sys->accounting_mem_sys = mem_sys;
       mem_sys->child_mem_sys = NULL;
  @@ -213,7 +185,7 @@
       return APR_SUCCESS;
   }
   
  -#ifdef APR_MEMORY_ASSERT
  +#ifdef APR_ASSERT_MEMORY
   APR_DECLARE(void) apr_sms_assert(apr_sms_t *mem_sys)
   {
       apr_sms_t *parent;
  @@ -249,10 +221,7 @@
        * tracking ancestors, but in that specific case we issue a
        * warning.
        */
  -    if (!mem_sys->parent_mem_sys)
  -        return;
  -
  -    parent = mem_sys
  +    parent = mem_sys->parent_mem_sys;
       while (parent) {
           if (apr_sms_is_tracking(parent))
               return; /* Tracking memory system found, return satisfied ;-) */
  @@ -265,7 +234,7 @@
        * parent.
        */
   }
  -#endif /* APR_MEMORY_ASSERT */
  +#endif /* APR_ASSERT_MEMORY */
   
   /*
    * LOCAL FUNCTION used in:
  @@ -312,9 +281,6 @@
   
   APR_DECLARE(apr_status_t) apr_sms_reset(apr_sms_t *mem_sys)
   {
  -    if (!mem_sys)
  -        return APR_EMEMSYS;
  -
       if (!mem_sys->reset_fn)
           return APR_ENOTIMPL;
   
  @@ -350,9 +316,6 @@
       struct apr_sms_cleanup *cleanup;
       struct apr_sms_cleanup *next_cleanup;
   
  -    if (!mem_sys)
  -        return APR_EMEMSYS;
  -
       if (apr_sms_is_tracking(mem_sys)) {
           /* 
            * Run the cleanups of all child memory systems _including_
  @@ -468,7 +431,7 @@
           mem_sys = mem_sys->parent_mem_sys;
       }
       assert(0); /* Made the wrong assumption, so we assert */
  -#endif /* APR_MEMORY_ASSERT */
  +#endif /* APR_ASSERT_MEMORY */
       
       return APR_SUCCESS;
   }
  @@ -476,9 +439,6 @@
   APR_DECLARE(apr_status_t) apr_sms_is_ancestor(apr_sms_t *a,
                                                 apr_sms_t *b)
   {
  -    if (!b)
  -        return APR_EMEMSYS;
  -
   #ifdef APR_ASSERT_MEMORY
       assert(a);
   #endif
  @@ -492,30 +452,18 @@
   
   APR_DECLARE(apr_status_t) apr_sms_lock(apr_sms_t *mem_sys)
   {
  -    if (!mem_sys)
  -        return APR_EMEMSYS;       
  -
       if (!mem_sys->lock_fn)
           return APR_ENOTIMPL;
   
  -    if (mem_sys->lock_fn)
  -        return mem_sys->lock_fn(mem_sys);
  -
  -    return APR_SUCCESS;
  +    return mem_sys->lock_fn(mem_sys);
   }
   
   APR_DECLARE(apr_status_t) apr_sms_unlock(apr_sms_t *mem_sys)
   {
  -    if (!mem_sys)
  -        return APR_EMEMSYS;
  -
       if (!mem_sys->unlock_fn)
           return APR_ENOTIMPL;
           
  -    if (mem_sys->unlock_fn)
  -        return mem_sys->unlock_fn(mem_sys);
  -
  -    return APR_SUCCESS;
  +    return mem_sys->unlock_fn(mem_sys);
   }
   
   /*
  @@ -530,13 +478,6 @@
   {
       struct apr_sms_cleanup *cleanup;
   
  -    if (!mem_sys)
  -        return APR_EMEMSYS;
  -
  -#ifdef APR_ASSERT_MEMORY
  -    assert(mem_sys->accounting_mem_sys);
  -#endif
  -    
       if (!cleanup_fn)
           return APR_ENOTIMPL;
   
  @@ -565,13 +506,6 @@
       struct apr_sms_cleanup *cleanup;
       struct apr_sms_cleanup **cleanup_ref;
   
  -    if (!mem_sys)
  -        return APR_EMEMSYS;
  -
  -#ifdef APR_ASSERT_MEMORY
  -    assert(mem_sys->accounting_mem_sys);
  -#endif
  -        
       cleanup = mem_sys->cleanups;
       cleanup_ref = &mem_sys->cleanups;
       while (cleanup) {
  @@ -591,8 +525,8 @@
           cleanup = cleanup->next;
       }
   
  -    /* The cleanup function should have been registered previously */
  -    return APR_ENOCLEANUP;
  +    /* The cleanup function must have been registered previously */
  +    return APR_EINVAL;
   }
   
   APR_DECLARE(apr_status_t) apr_sms_cleanup_unregister_type(apr_sms_t *mem_sys, 
  @@ -600,15 +534,8 @@
   {
       struct apr_sms_cleanup *cleanup;
       struct apr_sms_cleanup **cleanup_ref;
  -    apr_status_t rv = APR_ENOCLEANUP;
  +    apr_status_t rv = APR_EINVAL;
   
  -    if (!mem_sys)
  -        return APR_EMEMSYS;
  -        
  -#ifdef APR_ASSERT_MEMORY
  -    assert(mem_sys->accounting_mem_sys);
  -#endif
  -    
       cleanup = mem_sys->cleanups;
       cleanup_ref = &mem_sys->cleanups;
       mem_sys = mem_sys->accounting_mem_sys;
  @@ -628,7 +555,7 @@
           }
       }
   
  -    /* The cleanup function should have been registered previously */
  +    /* The cleanup function must have been registered previously */
       return rv;
   }
   
  @@ -640,14 +567,11 @@
   {
       apr_status_t rv;
   
  -    if (!mem_sys)
  -        return APR_EMEMSYS;
  -
       if ((rv = apr_sms_cleanup_unregister(mem_sys, type,
                                            data, cleanup_fn)) != APR_SUCCESS)
           return rv;
   
  -     return cleanup_fn(data);
  +    return cleanup_fn(data);
   }
   
   APR_DECLARE(apr_status_t) apr_sms_cleanup_run_type(apr_sms_t *mem_sys, 
  @@ -655,15 +579,8 @@
   {
       struct apr_sms_cleanup *cleanup;
       struct apr_sms_cleanup **cleanup_ref;
  -    apr_status_t rv = APR_ENOCLEANUP;
  +    apr_status_t rv = APR_ENOTIMPL;
   
  -    if (!mem_sys)
  -        return APR_EMEMSYS;
  -        
  -#ifdef APR_ASSERT_MEMORY
  -    assert(mem_sys->accounting_mem_sys);
  -#endif
  -    
       cleanup = mem_sys->cleanups;
       cleanup_ref = &mem_sys->cleanups;
       mem_sys = mem_sys->accounting_mem_sys;
  
  
  
  1.6       +0 -3      apr/memory/unix/apr_sms_std.c
  
  Index: apr_sms_std.c
  ===================================================================
  RCS file: /home/cvs/apr/memory/unix/apr_sms_std.c,v
  retrieving revision 1.5
  retrieving revision 1.6
  diff -u -d -u -r1.5 -r1.6
  --- apr_sms_std.c	2001/06/01 17:32:14	1.5
  +++ apr_sms_std.c	2001/06/04 23:09:34	1.6
  @@ -64,7 +64,6 @@
   #include "apr_private.h"
   #include "apr_sms.h"
   #include <stdlib.h>
  -#include <assert.h>
   
   static const char *module_identity = "STANDARD";
   
  @@ -109,8 +108,6 @@
   {
       apr_sms_t *new_mem_sys;
       apr_status_t rv;
  -
  -    assert(mem_sys);
   
       *mem_sys = NULL;
       /* We don't have a parent so we allocate the memory
  
  
  
  1.6       +0 -25     apr/memory/unix/apr_sms_tracking.c
  
  Index: apr_sms_tracking.c
  ===================================================================
  RCS file: /home/cvs/apr/memory/unix/apr_sms_tracking.c,v
  retrieving revision 1.5
  retrieving revision 1.6
  diff -u -d -u -r1.5 -r1.6
  --- apr_sms_tracking.c	2001/06/01 17:32:14	1.5
  +++ apr_sms_tracking.c	2001/06/04 23:09:34	1.6
  @@ -66,7 +66,6 @@
   #include "apr_sms.h"
   #include "apr_sms_tracking.h"
   #include <stdlib.h>
  -#include <assert.h>
   
   static const char *module_identity = "TRACKING";
   
  @@ -93,8 +92,6 @@
       apr_sms_tracking_t *tms;
       apr_track_node_t *node;
     
  -    assert(mem_sys);
  -
       tms = (apr_sms_tracking_t *)mem_sys;
       node = apr_sms_malloc(mem_sys->parent_mem_sys,
                             size + sizeof(apr_track_node_t));
  @@ -118,8 +115,6 @@
       apr_sms_tracking_t *tms;
       apr_track_node_t *node;
     
  -    assert(mem_sys);
  -
       tms = (apr_sms_tracking_t *)mem_sys;
       node = apr_sms_calloc(mem_sys->parent_mem_sys,
                             size + sizeof(apr_track_node_t));
  @@ -143,8 +138,6 @@
       apr_sms_tracking_t *tms;
       apr_track_node_t *node;
   
  -    assert(mem_sys);
  -
       tms = (apr_sms_tracking_t *)mem_sys;
       node = (apr_track_node_t *)mem;
   
  @@ -174,9 +167,6 @@
   {
       apr_track_node_t *node;
       
  -    assert(mem_sys);
  -    assert(mem);
  -
       node = (apr_track_node_t *)mem;
       node--;
   
  @@ -193,8 +183,6 @@
       apr_track_node_t *node;
       apr_status_t rv;
    
  -    assert(mem_sys);
  -
       tms = (apr_sms_tracking_t *)mem_sys;
   
       while (tms->nodes) {
  @@ -214,16 +202,6 @@
   {
       apr_status_t rv;
       
  -    /* If this is NULL we won't blow up as it should be caught at the
  -     * next level down and then passed back to us...
  -     */
  -#ifdef APR_ASSERT_MEMORY
  -    assert(mem_sys->parent_mem_sys);
  -#endif
  -    
  -    if (!mem_sys)
  -        return APR_EMEMSYS;
  -
       if ((rv = apr_sms_tracking_reset(mem_sys)) != APR_SUCCESS)
           return rv;
       
  @@ -237,9 +215,6 @@
       apr_sms_tracking_t *tms;
       apr_status_t rv;
   
  -    assert(mem_sys);
  -    assert(pms);
  -    
       *mem_sys = NULL;
       /* We're not a top level module, ie we have a parent, so
        * we allocate the memory for the structure from our parent.
  
  
  
  1.39      +0 -4      apr/misc/unix/errorcodes.c
  
  Index: errorcodes.c
  ===================================================================
  RCS file: /home/cvs/apr/misc/unix/errorcodes.c,v
  retrieving revision 1.38
  retrieving revision 1.39
  diff -u -d -u -r1.38 -r1.39
  --- errorcodes.c	2001/05/19 22:47:29	1.38
  +++ errorcodes.c	2001/06/04 23:09:36	1.39
  @@ -166,10 +166,6 @@
           return "The given path was above the root path";
       case APR_EBADPATH:
           return "The given path misformatted or contained invalid characters";
  -    case APR_EMEMSYS:
  -        return "The memory system passed does not exist";
  -    case APR_ENOCLEANUP:
  -        return "The requested cleanup function does not exist";
       default:
           return "Error string not specified yet";
       }
  
  
  

Re: cvs commit: apr/misc/unix errorcodes.c

Posted by "William A. Rowe, Jr." <ad...@rowe-clan.net>.
From: "Cliff Woolley" <cl...@yahoo.com>
Sent: Tuesday, June 05, 2001 12:36 PM


> > Yes, maybe the "no checking" isn't documented, but it has been policy for a
> > long while. Yes, we *should* document it. Nobody has got around to it is
> > all. I'm surprised that you aren't familiar with it.
> 
> If I knew where to put it, I'd document it.  I'm sure there are a million
> other unwritten "policies" that should go in the same place.  Anybody who
> can think of one of them, drop me a note and I'll put it on my "list of
> policies to document".

Checkout the apr-site and type away :-)  We have a big, blank slate to work with.

Bill




Re: cvs commit: apr/misc/unix errorcodes.c

Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Tue, Jun 05, 2001 at 10:46:52AM -0700, rbb@covalent.net wrote:
> If I can suggest a place, the Web site.  We already have a page about "How
> to contribute code", so just add it to that page.

If no one beats me to it, I'll look at adding this in the next few days.
Now that I know how to update the web site (thanks, Greg!).  

We can add the policies/standards as we come across them.  -- justin


Re: cvs commit: apr/misc/unix errorcodes.c

Posted by rb...@covalent.net.
On Tue, 5 Jun 2001, Cliff Woolley wrote:

> On Tue, 5 Jun 2001, Greg Stein wrote:
>
> > Ease up, David. Cliff did the right thing here.
>
> We all just got a little worked up, I think.  No harm done.  Case closed.
>
> > Yes, maybe the "no checking" isn't documented, but it has been policy for a
> > long while. Yes, we *should* document it. Nobody has got around to it is
> > all. I'm surprised that you aren't familiar with it.
>
> If I knew where to put it, I'd document it.  I'm sure there are a million
> other unwritten "policies" that should go in the same place.  Anybody who
> can think of one of them, drop me a note and I'll put it on my "list of
> policies to document".

If I can suggest a place, the Web site.  We already have a page about "How
to contribute code", so just add it to that page.

Ryan

_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------


Re: cvs commit: apr/misc/unix errorcodes.c

Posted by Cliff Woolley <cl...@yahoo.com>.
On Tue, 5 Jun 2001, Greg Stein wrote:

> Ease up, David. Cliff did the right thing here.

We all just got a little worked up, I think.  No harm done.  Case closed.

> Yes, maybe the "no checking" isn't documented, but it has been policy for a
> long while. Yes, we *should* document it. Nobody has got around to it is
> all. I'm surprised that you aren't familiar with it.

If I knew where to put it, I'd document it.  I'm sure there are a million
other unwritten "policies" that should go in the same place.  Anybody who
can think of one of them, drop me a note and I'll put it on my "list of
policies to document".

--Cliff


--------------------------------------------------------------
   Cliff Woolley
   cliffwoolley@yahoo.com
   Charlottesville, VA



Re: cvs commit: apr/misc/unix errorcodes.c

Posted by Greg Stein <gs...@lyra.org>.
Ease up, David. Cliff did the right thing here.

Yes, maybe the "no checking" isn't documented, but it has been policy for a
long while. Yes, we *should* document it. Nobody has got around to it is
all. I'm surprised that you aren't familiar with it.

Cheers,
-g

On Tue, Jun 05, 2001 at 09:23:08AM +0100, David Reid wrote:
> >   * Remove the unnecessary parameter checks and the extra error codes that
> >     went along with them.  The APR policy is to segfault on a NULL
> parameter
> >     rather than silently returning some error code that the caller might
> >     not check anyway.
> 
> Can't say I agree 100% with this, but if you say so, it must be.  BTW, where
> is this written/documented??
> 
> >   * Also remove lots of unnecessary assertions (where the code would have
> >     segfaulted anyway, even without an explicit assert).  I've tried to be
> >     sure that every one I removed will result in a virtually immediate
> >     segfault anyway.  Ones that don't are the ones that are tricky to
> debug.
> >     If I've removed too many, say so and I'll put them back.
> 
> Whatever...
> 
> >   * Fix a misnamed APR_MEMORY_ASSERT -> APR_ASSERT_MEMORY, which was
> causing
> >     apr_assert_memory() never to be compiled.  Also fix a syntax error in
> that
> >     function that's been there since rev 1.1 of apr_sms.c, which no one's
> >     ever noticed because they never compiled it before.
> 
> If you checked you'd see that the misnamed assert tag was added in revision
> 1.5 of the original file, and so the code that you claim was never built was
> building quite happily up till that point.  As for the screw up on the
> naming, don't believe I missed that!
> 
> david

-- 
Greg Stein, http://www.lyra.org/

Re: cvs commit: apr/misc/unix errorcodes.c

Posted by Cliff Woolley <cl...@yahoo.com>.
On Tue, 5 Jun 2001, David Reid wrote:

> >   * Remove the unnecessary parameter checks and the extra error codes that
> >     went along with them.  The APR policy is to segfault on a NULL
> >     parameter
> >     rather than silently returning some error code that the caller might
> >     not check anyway.
>
> Can't say I agree 100% with this, but if you say so, it must be.  BTW, where
> is this written/documented??

I don't know if it IS documented, but I've been told this a thousand times
by various people since I came on board, and I've never once been told
the opposite.  <shrug>


> >   * Fix a misnamed APR_MEMORY_ASSERT -> APR_ASSERT_MEMORY, which was
> >     causing
> >     apr_assert_memory() never to be compiled.  Also fix a syntax error in

s/apr_assert_memory/apr_sms_assert/

So sue me.


> >     that
> >     function that's been there since rev 1.1 of apr_sms.c, which no one's
> >     ever noticed because they never compiled it before.
>
> If you checked you'd see that the misnamed assert tag was added in revision
> 1.5 of the original file, and so the code that you claim was never built was
> building quite happily up till that point.  As for the screw up on the
> naming, don't believe I missed that!

Huh?  I did check.  The misnamed tag and the missing semicolon had both
been there since rev 1.1 of apr_sms.c.  Which file are you talking about?
So anyway, apr_sms.c was using both APR_MEMORY_ASSERT and
APR_ASSERT_MEMORY since rev 1.1, but only APR_ASSERT_MEMORY was ever
actually defined by configure.  <shrug>  Oh well.  No big deal.  It's
fixed now.

Look, I wasn't trying to irritate anybody with this commit (or commit
message, for that matter).  The commit message was only meant to point out
that I'd found a typo and tried to fix it so that somebody would know to
look closely at that part of the commit.  Sander did so, found that I'd in
fact made a mistake, and I fixed it.  That's what this is all about.  As
for the commit, I was just trying to implement what seemed to be the
collective will of the group given what's been said by Greg, Jeff, Justin,
myself, and others.  That's all.  If it came off as a personal thing, I
apologize... that wasn't the intent.

--Cliff


--------------------------------------------------------------
   Cliff Woolley
   cliffwoolley@yahoo.com
   Charlottesville, VA




Re: cvs commit: apr/misc/unix errorcodes.c

Posted by David Reid <dr...@jetnet.co.uk>.
>   * Remove the unnecessary parameter checks and the extra error codes that
>     went along with them.  The APR policy is to segfault on a NULL
parameter
>     rather than silently returning some error code that the caller might
>     not check anyway.

Can't say I agree 100% with this, but if you say so, it must be.  BTW, where
is this written/documented??

>   * Also remove lots of unnecessary assertions (where the code would have
>     segfaulted anyway, even without an explicit assert).  I've tried to be
>     sure that every one I removed will result in a virtually immediate
>     segfault anyway.  Ones that don't are the ones that are tricky to
debug.
>     If I've removed too many, say so and I'll put them back.

Whatever...

>   * Fix a misnamed APR_MEMORY_ASSERT -> APR_ASSERT_MEMORY, which was
causing
>     apr_assert_memory() never to be compiled.  Also fix a syntax error in
that
>     function that's been there since rev 1.1 of apr_sms.c, which no one's
>     ever noticed because they never compiled it before.

If you checked you'd see that the misnamed assert tag was added in revision
1.5 of the original file, and so the code that you claim was never built was
building quite happily up till that point.  As for the screw up on the
naming, don't believe I missed that!

david



RE: cvs commit: apr/misc/unix errorcodes.c

Posted by Cliff Woolley <cl...@yahoo.com>.
On Tue, 5 Jun 2001, Sander Striker wrote:

> >   -    if (!mem_sys->parent_mem_sys)
> >   -        return;
> >   -
> >   -    parent = mem_sys
> >   +    parent = mem_sys->parent_mem_sys;
>
> This line should read:
>          parent = mem_sys;
>
> The condition is also satified if the memory system itself is
> tracking. The naming may be confusing, but this is how it
> should be.

I wondered about that and was going to ask.  Thanks.  Commit coming.  =-)

--Cliff


--------------------------------------------------------------
   Cliff Woolley
   cliffwoolley@yahoo.com
   Charlottesville, VA



RE: cvs commit: apr/misc/unix errorcodes.c

Posted by Sander Striker <st...@samba-tng.org>.
Hi,

Thanks for bringing the code more into APR policy I guess.

>   -#ifdef APR_MEMORY_ASSERT
>   +#ifdef APR_ASSERT_MEMORY
>    APR_DECLARE(void) apr_sms_assert(apr_sms_t *mem_sys)
>    {
>        apr_sms_t *parent;
>   @@ -249,10 +221,7 @@
>         * tracking ancestors, but in that specific case we issue a
>         * warning.
>         */
>   -    if (!mem_sys->parent_mem_sys)
>   -        return;
>   -
>   -    parent = mem_sys
>   +    parent = mem_sys->parent_mem_sys;

This line should read:
         parent = mem_sys;

The condition is also satified if the memory system itself is
tracking. The naming may be confusing, but this is how it
should be.

>        while (parent) {
>            if (apr_sms_is_tracking(parent))
>                return; /* Tracking memory system found, return 
> satisfied ;-) */

Sander