tal: support destructors with an extra argument.

There are several times I've wanted an extra arg to the destructor, and had
to embed it in the thing destroyed.  It's more efficient to put it into
tal itself (since it allocates space anyway), but we make it conditional
on a flag to avoid bloating every destructor.

The infrastructure makes it easier to add an extra arg to the general
notifiers later if we want.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

ccan/tal/tal.c

@@ -14,6 +14,7 @@
 //#define TAL_DEBUG 1
 
 #define NOTIFY_IS_DESTRUCTOR 512
+#define NOTIFY_EXTRA_ARG 1024
 
 /* 32-bit type field, first byte 0 in either endianness. */
 enum prop_type {
@@ -56,9 +57,18 @@ struct notifier {
 	union {
 		void (*notifyfn)(tal_t *, enum tal_notify_type, void *);
 		void (*destroy)(tal_t *); /* If NOTIFY_IS_DESTRUCTOR set */
+		void (*destroy2)(tal_t *, void *); /* If NOTIFY_EXTRA_ARG */
 	} u;
 };
 
+/* Extra arg */
+struct notifier_extra_arg {
+	struct notifier n;
+	void *arg;
+};
+
+#define EXTRA_ARG(n) (((struct notifier_extra_arg *)(n))->arg)
+
 static struct {
 	struct tal_hdr hdr;
 	struct children c;
@@ -218,9 +228,13 @@ static void notify(const struct tal_hdr *ctx,
 		n = (struct notifier *)p;
 		if (n->types & type) {
 			errno = saved_errno;
-			if (n->types & NOTIFY_IS_DESTRUCTOR)
-				n->u.destroy(from_tal_hdr(ctx));
-			else
+			if (n->types & NOTIFY_IS_DESTRUCTOR) {
+				if (n->types & NOTIFY_EXTRA_ARG)
+					n->u.destroy2(from_tal_hdr(ctx),
+						      EXTRA_ARG(n));
+				else
+					n->u.destroy(from_tal_hdr(ctx));
+			} else
 				n->u.notifyfn(from_tal_hdr(ctx), type,
 					      (void *)info);
 		}
@@ -276,13 +290,22 @@ static struct notifier *add_notifier_property(struct tal_hdr *t,
 					      enum tal_notify_type types,
 					      void (*fn)(void *,
 							 enum tal_notify_type,
-							 void *))
+							 void *),
+					      void *extra_arg)
 {
-	struct notifier *prop = allocate(sizeof(*prop));
+	struct notifier *prop;
+
+	if (types & NOTIFY_EXTRA_ARG)
+		prop = allocate(sizeof(struct notifier_extra_arg));
+	else
+		prop = allocate(sizeof(struct notifier));
+
 	if (prop) {
 		init_property(&prop->hdr, t, NOTIFIER);
 		prop->types = types;
 		prop->u.notifyfn = fn;
+		if (types & NOTIFY_EXTRA_ARG)
+			EXTRA_ARG(prop) = extra_arg;
 	}
 	return prop;
 }
@@ -290,24 +313,33 @@ static struct notifier *add_notifier_property(struct tal_hdr *t,
 static enum tal_notify_type del_notifier_property(struct tal_hdr *t,
 						  void (*fn)(tal_t *,
 							     enum tal_notify_type,
-							     void *))
+							     void *),
+						  bool match_extra_arg,
+						  void *extra_arg)
 {
         struct prop_hdr **p;
 
         for (p = (struct prop_hdr **)&t->prop; *p; p = &(*p)->next) {
 		struct notifier *n;
+		enum tal_notify_type types;
 
                 if (is_literal(*p))
 			break;
                 if ((*p)->type != NOTIFIER)
 			continue;
 		n = (struct notifier *)*p;
-		if (n->u.notifyfn == fn) {
-			enum tal_notify_type types = n->types;
-			*p = (*p)->next;
-			freefn(n);
-			return types & ~NOTIFY_IS_DESTRUCTOR;
-		}
+		if (n->u.notifyfn != fn)
+			continue;
+
+		types = n->types;
+		if ((types & NOTIFY_EXTRA_ARG)
+		    && match_extra_arg
+		    && extra_arg != EXTRA_ARG(n))
+			continue;
+
+		*p = (*p)->next;
+		freefn(n);
+		return types & ~(NOTIFY_IS_DESTRUCTOR|NOTIFY_EXTRA_ARG);
         }
         return 0;
 }
@@ -506,9 +538,19 @@ bool tal_add_destructor_(const tal_t *ctx, void (*destroy)(void *me))
 {
 	tal_t *t = debug_tal(to_tal_hdr(ctx));
 	return add_notifier_property(t, TAL_NOTIFY_FREE|NOTIFY_IS_DESTRUCTOR,
-				     (void *)destroy);
+				     (void *)destroy, NULL);
+}
+
+bool tal_add_destructor2_(const tal_t *ctx, void (*destroy)(void *me, void *arg),
+			  void *arg)
+{
+	tal_t *t = debug_tal(to_tal_hdr(ctx));
+	return add_notifier_property(t, TAL_NOTIFY_FREE|NOTIFY_IS_DESTRUCTOR
+				     |NOTIFY_EXTRA_ARG,
+				     (void *)destroy, arg);
 }
 
+/* We could support notifiers with an extra arg, but we didn't add to API */
 bool tal_add_notifier_(const tal_t *ctx, enum tal_notify_type types,
 		       void (*callback)(tal_t *, enum tal_notify_type, void *))
 {
@@ -523,7 +565,7 @@ bool tal_add_notifier_(const tal_t *ctx, enum tal_notify_type types,
 			  | TAL_NOTIFY_DEL_NOTIFIER)) == 0);
 
 	/* Don't call notifier about itself: set types after! */
-        n = add_notifier_property(t, 0, callback);
+        n = add_notifier_property(t, 0, callback, NULL);
 	if (unlikely(!n))
 		return false;
 
@@ -537,12 +579,13 @@ bool tal_add_notifier_(const tal_t *ctx, enum tal_notify_type types,
 }
 
 bool tal_del_notifier_(const tal_t *ctx,
-		       void (*callback)(tal_t *, enum tal_notify_type, void *))
+		       void (*callback)(tal_t *, enum tal_notify_type, void *),
+		       bool match_extra_arg, void *extra_arg)
 {
 	struct tal_hdr *t = debug_tal(to_tal_hdr(ctx));
 	enum tal_notify_type types;
 
-        types = del_notifier_property(t, callback);
+        types = del_notifier_property(t, callback, match_extra_arg, extra_arg);
 	if (types) {
 		notify(t, TAL_NOTIFY_DEL_NOTIFIER, callback, 0);
 		if (types != TAL_NOTIFY_FREE)
@@ -554,7 +597,13 @@ bool tal_del_notifier_(const tal_t *ctx,
 
 bool tal_del_destructor_(const tal_t *ctx, void (*destroy)(void *me))
 {
-	return tal_del_notifier_(ctx, (void *)destroy);
+	return tal_del_notifier_(ctx, (void *)destroy, false, NULL);
+}
+
+bool tal_del_destructor2_(const tal_t *ctx, void (*destroy)(void *me, void *arg),
+			  void *arg)
+{
+	return tal_del_notifier_(ctx, (void *)destroy, true, arg);
 }
 
 bool tal_set_name_(tal_t *ctx, const char *name, bool literal)

ccan/tal/tal.h

@@ -169,6 +169,53 @@ void *tal_free(const tal_t *p);
 #define tal_del_destructor(ptr, function)				      \
 	tal_del_destructor_((ptr), typesafe_cb(void, void *, (function), (ptr)))
 
+/**
+ * tal_add_destructor2 - add a 2-arg callback function when context is destroyed.
+ * @ptr: The tal allocated object.
+ * @function: the function to call before it's freed.
+ * @arg: the extra argument to the function.
+ *
+ * Sometimes an extra argument is required for a destructor; this
+ * saves the extra argument internally to avoid the caller having to
+ * do an extra allocation.
+ *
+ * Note that this can only fail if your allocfn fails and your errorfn returns.
+ */
+#define tal_add_destructor2(ptr, function, arg)				\
+	tal_add_destructor2_((ptr),					\
+			     typesafe_cb_cast(void (*)(tal_t *, void *), \
+					      void (*)(__typeof__(ptr), \
+						       __typeof__(arg)), \
+					      (function)),		\
+			     (arg))
+
+/**
+ * tal_del_destructor - remove a destructor callback function.
+ * @ptr: The tal allocated object.
+ * @function: the function to call before it's freed.
+ *
+ * If @function has not been successfully added as a destructor, this returns
+ * false.
+ */
+#define tal_del_destructor(ptr, function)				      \
+	tal_del_destructor_((ptr), typesafe_cb(void, void *, (function), (ptr)))
+
+/**
+ * tal_del_destructor2 - remove 2-arg callback function.
+ * @ptr: The tal allocated object.
+ * @function: the function to call before it's freed.
+ * @arg: the extra argument to the function.
+ *
+ * If @function has not been successfully added as a destructor with
+ * @arg, this returns false.
+ */
+#define tal_del_destructor2(ptr, function, arg)				\
+	tal_del_destructor2_((ptr),					\
+			     typesafe_cb_cast(void (*)(tal_t *, void *), \
+					      void (*)(__typeof__(ptr), \
+						       __typeof__(arg)), \
+					      (function)),		\
+			     (arg))
 enum tal_notify_type {
 	TAL_NOTIFY_FREE = 1,
 	TAL_NOTIFY_STEAL = 2,
@@ -234,7 +281,8 @@ enum tal_notify_type {
 	tal_del_notifier_((ptr),					\
 			  typesafe_cb_postargs(void, void *, (callback), \
 					       (ptr),			\
-					       enum tal_notify_type, void *))
+					       enum tal_notify_type, void *), \
+			  false, NULL)
 
 /**
  * tal_set_name - attach a name to a tal pointer.
@@ -449,12 +497,17 @@ bool tal_resize_(tal_t **ctxp, size_t size, size_t count, bool clear);
 bool tal_expand_(tal_t **ctxp, const void *src, size_t size, size_t count);
 
 bool tal_add_destructor_(const tal_t *ctx, void (*destroy)(void *me));
+bool tal_add_destructor2_(const tal_t *ctx, void (*destroy)(void *me, void *arg),
+			  void *arg);
 bool tal_del_destructor_(const tal_t *ctx, void (*destroy)(void *me));
+bool tal_del_destructor2_(const tal_t *ctx, void (*destroy)(void *me, void *arg),
+			  void *arg);
 
 bool tal_add_notifier_(const tal_t *ctx, enum tal_notify_type types,
 		       void (*notify)(tal_t *ctx, enum tal_notify_type,
 				      void *info));
 bool tal_del_notifier_(const tal_t *ctx,
 		       void (*notify)(tal_t *ctx, enum tal_notify_type,
-				      void *info));
+				      void *info),
+		       bool match_extra_arg, void *arg);
 #endif /* CCAN_TAL_H */

ccan/tal/test/run-destructor2.c

+#include <ccan/tal/tal.h>
+#include <ccan/tal/tal.c>
+#include <ccan/tap/tap.h>
+
+static void destroy_inc(char *p UNNEEDED, int *destroy_count)
+{
+	(*destroy_count)++;
+}
+
+static void destroy_dec(char *p UNNEEDED, int *destroy_count)
+{
+	(*destroy_count)--;
+}
+
+int main(void)
+{
+	char *p;
+	int destroy_count1 = 0, destroy_count2 = 0;
+
+	plan_tests(10);
+
+	p = tal(NULL, char);
+	/* Del must match both fn and arg. */
+	ok1(tal_add_destructor2(p, destroy_inc, &destroy_count1));
+	ok1(!tal_del_destructor2(p, destroy_inc, &destroy_count2));
+	ok1(!tal_del_destructor2(p, destroy_dec, &destroy_count1));
+	ok1(tal_del_destructor2(p, destroy_inc, &destroy_count1));
+	ok1(!tal_del_destructor2(p, destroy_inc, &destroy_count1));
+
+	ok1(tal_add_destructor2(p, destroy_inc, &destroy_count1));
+	ok1(tal_add_destructor2(p, destroy_dec, &destroy_count2));
+	ok1(tal_free(p) == NULL);
+	ok1(destroy_count1 == 1);
+	ok1(destroy_count2 == -1);
+
+	tal_cleanup();
+	return exit_status();
+}