Re: Debugging case refs

[Ben Pfaff]

Thanks.  I pushed this.

John Darrington <address@hidden> writes:

> Looks ok to me.  I've not tried it.
> Let's check it in and see.....
>
> On Tue, Oct 26, 2010 at 07:35:53AM -0700, Ben Pfaff wrote:
>      John Darrington <address@hidden> writes:
>      
>      > On Mon, Oct 25, 2010 at 09:57:47PM -0700, Ben Pfaff wrote:
>      >      
>      >      From: Ben Pfaff <address@hidden>
>      >      Date: Mon, 25 Oct 2010 21:56:48 -0700
>      >      Subject: [PATCH] case: Add support for debugging case reference 
> count leaks.
>      >       
>      >      -static inline struct ccase *case_ref (const struct ccase *);
>      >      +struct ccase *case_ref (const struct ccase *);
>      >
>      > I think it's important that the case_ref function has the 
> WARN_UNUSED_RESULT flag.
>      > When DEBUG_CASEREFS=1, simply calling case_ref (c) without assigning 
> the result
>      > will not copy the case.
>      
>      Good point, thanks.
>      
>      Here's the same patch with that one change:
>      
>      --8<--------------------------cut here-------------------------->8--
>      
>      From: Ben Pfaff <address@hidden>
>      Date: Tue, 26 Oct 2010 07:34:45 -0700
>      Subject: [PATCH] case: Add support for debugging case reference count 
> leaks.
>      
>      ---
>       src/data/case.c |   23 ++++++++++++++++++++++-
>       src/data/case.h |   14 ++------------
>       2 files changed, 24 insertions(+), 13 deletions(-)
>      
>      diff --git a/src/data/case.c b/src/data/case.c
>      index dc40292..ffb47fe 100644
>      --- a/src/data/case.c
>      +++ b/src/data/case.c
>      @@ -1,5 +1,5 @@
>       /* PSPP - a program for statistical analysis.
>      -   Copyright (C) 2004, 2007, 2009 Free Software Foundation, Inc.
>      +   Copyright (C) 2004, 2007, 2009, 2010 Free Software Foundation, Inc.
>       
>          This program is free software: you can redistribute it and/or modify
>          it under the terms of the GNU General Public License as published by
>      @@ -30,6 +30,14 @@
>       #include "minmax.h"
>       #include "xalloc.h"
>       
>      +/* Set this flag to 1 to copy cases instead of ref counting them.
>      +   This is sometimes helpful in debugging situations. */
>      +#define DEBUG_CASEREFS 0
>      +
>      +#if DEBUG_CASEREFS
>      +#warning "Caseref debug enabled.  CASES ARE NOT BEING SHARED!!"
>      +#endif
>      +
>       static size_t case_size (const struct caseproto *);
>       static bool variable_matches_case (const struct ccase *,
>                                          const struct variable *);
>      @@ -80,6 +88,19 @@ case_clone (const struct ccase *c)
>         return case_unshare (case_ref (c));
>       }
>       
>      +/* Increments case C's reference count and returns C.  Afterward,
>      +   case C is shared among its reference count holders. */
>      +struct ccase *
>      +case_ref (const struct ccase *c_)
>      +{
>      +  struct ccase *c = CONST_CAST (struct ccase *, c_);
>      +  c->ref_cnt++;
>      +#if DEBUG_CASEREFS
>      +  c = case_unshare__ (c);
>      +#endif
>      +  return c;
>      +}
>      +
>       /* Returns an estimate of the number of bytes of memory that
>          would be consumed in creating a case based on PROTO.  The
>          estimate includes typical overhead from malloc() in addition
>      diff --git a/src/data/case.h b/src/data/case.h
>      index d00c2ad..c07b07d 100644
>      --- a/src/data/case.h
>      +++ b/src/data/case.h
>      @@ -1,5 +1,5 @@
>       /* PSPP - a program for statistical analysis.
>      -   Copyright (C) 2004, 2007, 2009 Free Software Foundation, Inc.
>      +   Copyright (C) 2004, 2007, 2009, 2010 Free Software Foundation, Inc.
>       
>          This program is free software: you can redistribute it and/or modify
>          it under the terms of the GNU General Public License as published by
>      @@ -61,7 +61,7 @@ struct ccase *case_try_create (const struct caseproto 
> *) MALLOC_LIKE;
>       struct ccase *case_clone (const struct ccase *) MALLOC_LIKE;
>       
>       static inline struct ccase *case_unshare (struct ccase *) 
> WARN_UNUSED_RESULT;
>      -static inline struct ccase *case_ref (const struct ccase *);
>      +struct ccase *case_ref (const struct ccase *) WARN_UNUSED_RESULT;
>       static inline void case_unref (struct ccase *);
>       static inline bool case_is_shared (const struct ccase *);
>       
>      @@ -130,16 +130,6 @@ case_unshare (struct ccase *c)
>         return c;
>       }
>       
>      -/* Increments case C's reference count and returns C.  Afterward,
>      -   case C is shared among its reference count holders. */
>      -static inline struct ccase *
>      -case_ref (const struct ccase *c_)
>      -{
>      -  struct ccase *c = CONST_CAST (struct ccase *, c_);
>      -  c->ref_cnt++;
>      -  return c;
>      -}
>      -
>       /* Decrements case C's reference count.  Frees C if its
>          reference count drops to 0.
>       
>      -- 
>      Ben Pfaff 
>      http://benpfaff.org

-- 
Ben Pfaff 
http://benpfaff.org