OK. How about this new patch, which keeps the switch entirely within case.c
J'
On Mon, Oct 04, 2010 at 12:42:03PM -0700, Ben Pfaff wrote:
John Darrington <[email protected]> writes:
> It's almost impossible to track down memory leaks when ref counted
> objects are involved. I'm proposing this patch to case.[ch] which
> allows a preprocessor option to be set redefining case_ref and
case_unref
> to unconditionally copy and destroy cases respectively.
>
> It would be easier to use if we could put the switch entirely inside
case.c,
> since almost everything depends upon case.h -- but we could only do that
> if we sacrificed the benefits of the inline functions.
I think that this is valuable enough that we should move it into
case.c. I'm half-inclined to think that we should make it
configurable at runtime instead of at compilation time, but once
it is isolated in case.c this is less important because only a
single file will have to be recompiled if the setting changes.
--
Ben Pfaff
http://benpfaff.org
--
PGP Public key ID: 1024D/2DE827B3
fingerprint = 8797 A26D 0854 2EAB 0285 A290 8A67 719C 2DE8 27B3
See http://pgp.mit.edu or any PGP keyserver for public key.
diff --git a/src/data/case.c b/src/data/case.c
index dc40292..c577543 100644
--- a/src/data/case.c
+++ b/src/data/case.c
@@ -30,6 +30,25 @@
#include "minmax.h"
#include "xalloc.h"
+/*
+ Enable this flag 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
+
+struct ccase
+ {
+ struct caseproto *proto; /* Case prototype. */
+#if ! DEBUG_CASEREFS
+ size_t ref_cnt; /* Reference count. */
+#endif
+ union value values[1]; /* Values. */
+ };
+
static size_t case_size (const struct caseproto *);
static bool variable_matches_case (const struct ccase *,
const struct variable *);
@@ -65,7 +84,9 @@ case_try_create (const struct caseproto *proto)
if (caseproto_try_init_values (proto, c->values))
{
c->proto = caseproto_ref (proto);
+#if ! DEBUG_CASEREFS
c->ref_cnt = 1;
+#endif
return c;
}
free (c);
@@ -73,6 +94,53 @@ case_try_create (const struct caseproto *proto)
return NULL;
}
+/* 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_)
+{
+#if DEBUG_CASEREFS
+ return case_unshare__ (CONST_CAST (struct ccase *,c_));
+#else
+ struct ccase *c = CONST_CAST (struct ccase *, c_);
+ c->ref_cnt++;
+ return c;
+#endif
+}
+
+/* Decrements case C's reference count. Frees C if its
+ reference count drops to 0.
+
+ If C is a null pointer, this function has no effect. */
+void
+case_unref (struct ccase *c)
+{
+ if (c != NULL)
+#if ! DEBUG_CASEREFS
+ if (!--c->ref_cnt)
+#endif
+ case_unref__ (c);
+}
+
+
+/* Returns true if case C is shared. A case that is shared
+ cannot be modified directly. Instead, an unshared copy must
+ first be made with case_unshare(). */
+#if DEBUG_CASEREFS
+bool
+case_is_shared (const struct ccase *c UNUSED)
+{
+ return false;
+}
+#else
+bool
+case_is_shared (const struct ccase *c)
+{
+ return c->ref_cnt > 1;
+}
+#endif
+
+
/* Creates and returns an unshared copy of case C. */
struct ccase *
case_clone (const struct ccase *c)
@@ -148,7 +216,9 @@ case_unshare_and_resize (struct ccase *c, const struct caseproto *proto)
size_t old_n_values = caseproto_get_n_widths (c->proto);
size_t new_n_values = caseproto_get_n_widths (proto);
case_copy (new, 0, c, 0, MIN (old_n_values, new_n_values));
+#if ! DEBUG_CASEREFS
c->ref_cnt--;
+#endif
return new;
}
}
@@ -420,7 +490,9 @@ case_unshare__ (struct ccase *old)
{
struct ccase *new = case_create (old->proto);
case_copy (new, 0, old, 0, caseproto_get_n_widths (new->proto));
+#if ! DEBUG_CASEREFS
--old->ref_cnt;
+#endif
return new;
}
@@ -481,3 +553,17 @@ copy_backward (struct ccase *dst, size_t dst_idx,
caseproto_get_width (dst->proto, dst_idx + i));
}
+/* Returns the number of union values in C. */
+size_t
+case_get_value_cnt (const struct ccase *c)
+{
+ return caseproto_get_n_widths (c->proto);
+}
+
+/* Returns the prototype that describes the format of case C.
+ The caller must not unref the returned prototype. */
+const struct caseproto *
+case_get_proto (const struct ccase *c)
+{
+ return c->proto;
+}
diff --git a/src/data/case.h b/src/data/case.h
index d00c2ad..5908564 100644
--- a/src/data/case.h
+++ b/src/data/case.h
@@ -49,24 +49,19 @@ typedef long int casenumber;
case, this is undesirable behavior. The case_unshare function
provides a solution, by making a new, unshared copy of a
shared case. */
-struct ccase
- {
- struct caseproto *proto; /* Case prototype. */
- size_t ref_cnt; /* Reference count. */
- union value values[1]; /* Values. */
- };
+struct ccase;
struct ccase *case_create (const struct caseproto *) MALLOC_LIKE;
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 *);
-static inline void case_unref (struct ccase *);
-static inline bool case_is_shared (const struct ccase *);
+struct ccase *case_ref (const struct ccase *) WARN_UNUSED_RESULT;
+void case_unref (struct ccase *);
+bool case_is_shared (const struct ccase *);
-static inline size_t case_get_value_cnt (const struct ccase *);
-static inline const struct caseproto *case_get_proto (const struct ccase *);
+size_t case_get_value_cnt (const struct ccase *);
+const struct caseproto *case_get_proto (const struct ccase *);
size_t case_get_cost (const struct caseproto *);
@@ -130,49 +125,7 @@ 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.
- If C is a null pointer, this function has no effect. */
-static inline void
-case_unref (struct ccase *c)
-{
- if (c != NULL && !--c->ref_cnt)
- case_unref__ (c);
-}
-
-/* Returns true if case C is shared. A case that is shared
- cannot be modified directly. Instead, an unshared copy must
- first be made with case_unshare(). */
-static inline bool
-case_is_shared (const struct ccase *c)
-{
- return c->ref_cnt > 1;
-}
-
-/* Returns the number of union values in C. */
-static inline size_t
-case_get_value_cnt (const struct ccase *c)
-{
- return caseproto_get_n_widths (c->proto);
-}
-
-/* Returns the prototype that describes the format of case C.
- The caller must not unref the returned prototype. */
-static inline const struct caseproto *
-case_get_proto (const struct ccase *c)
-{
- return c->proto;
-}
#endif /* data/case.h */
signature.asc
Description: Digital signature
_______________________________________________ pspp-dev mailing list [email protected] http://lists.gnu.org/mailman/listinfo/pspp-dev
