From: Alan T. DeKok <aland@freeradius.org>
Date: Fri, 11 Sep 2015 16:10:35 +0000 (-0400)
Subject: Add fr_pair_list_mcopy_by_num()
X-Git-Tag: release_3_0_10~120
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=87ad059bbfb1cc703ca63f13c6b7f8b657d594f7;p=thirdparty%2Ffreeradius-server.git

Add fr_pair_list_mcopy_by_num()

Which is like fr_pair_list_move(), but does copy / delete
instead of talloc_steal.

The problem is that talloc_steal() keeps the original parent
context around for the lifetime of the VP being stolen.  Which is
bad when the VP comes from a REQUEST, and is put into another
context, which lives for multiple seconds.
---

diff --git a/src/include/libradius.h b/src/include/libradius.h
index 845eec6973f..48f46e185a6 100644
--- a/src/include/libradius.h
+++ b/src/include/libradius.h
@@ -632,7 +632,9 @@ void		fr_pair_value_bstrncpy(VALUE_PAIR *vp, void const * src, size_t len);
 void		fr_pair_value_sprintf(VALUE_PAIR *vp, char const * fmt, ...) CC_HINT(format (printf, 2, 3));
 void		fr_pair_list_move(TALLOC_CTX *ctx, VALUE_PAIR **to, VALUE_PAIR **from);
 void		fr_pair_list_move_by_num(TALLOC_CTX *ctx, VALUE_PAIR **to, VALUE_PAIR **from,
-			   unsigned int attr, unsigned int vendor, int8_t tag);
+					 unsigned int attr, unsigned int vendor, int8_t tag);
+void		fr_pair_list_mcopy_by_num(TALLOC_CTX *ctx, VALUE_PAIR **to, VALUE_PAIR **from,
+					  unsigned int attr, unsigned int vendor, int8_t tag);
 VALUE_PAIR	*fr_pair_afrom_ip_str(TALLOC_CTX *ctx, char const *value,
 			     DICT_ATTR *ipv4, DICT_ATTR *ipv6, DICT_ATTR *ipv4_prefix, DICT_ATTR *ipv6_prefix);
 int		fr_pair_value_from_str(VALUE_PAIR *vp, char const *value, size_t len);
diff --git a/src/lib/pair.c b/src/lib/pair.c
index fbdcf73ec5e..6a5900b6b10 100644
--- a/src/lib/pair.c
+++ b/src/lib/pair.c
@@ -935,8 +935,6 @@ void fr_pair_list_move(TALLOC_CTX *ctx, VALUE_PAIR **to, VALUE_PAIR **from)
  * Move pairs of a matching attribute number, vendor number and tag from the
  * the input list to the output list.
  *
- * @note pairs which are moved have their parent changed to ctx.
- *
  * @note fr_pair_list_free should be called on the head of the old list to free unmoved
 	 attributes (if they're no longer needed).
  *
@@ -949,11 +947,13 @@ void fr_pair_list_move(TALLOC_CTX *ctx, VALUE_PAIR **to, VALUE_PAIR **from)
  *	attributes.
  * @param[in] vendor to match.
  * @param[in] tag to match, TAG_ANY matches any tag, TAG_NONE matches tagless VPs.
+ * @param[in] move if set to "true", VPs are moved.  If set to "false", VPs are copied, and the old one deleted.
  */
-void fr_pair_list_move_by_num(TALLOC_CTX *ctx, VALUE_PAIR **to, VALUE_PAIR **from,
-			      unsigned int attr, unsigned int vendor, int8_t tag)
+static void fr_pair_list_move_by_num_internal(TALLOC_CTX *ctx, VALUE_PAIR **to, VALUE_PAIR **from,
+					      unsigned int attr, unsigned int vendor, int8_t tag,
+					      bool move)
 {
-	VALUE_PAIR *to_tail, *i, *next;
+	VALUE_PAIR *to_tail, *i, *next, *this;
 	VALUE_PAIR *iprev = NULL;
 
 	/*
@@ -1037,19 +1037,85 @@ void fr_pair_list_move_by_num(TALLOC_CTX *ctx, VALUE_PAIR **to, VALUE_PAIR **fro
 		else
 			*from = next;
 
+		if (move) {
+			this = i;
+		} else {
+			this = fr_pair_copy(ctx, i);
+		}
+
 		/*
 		 *	Add the attribute to the "to" list.
 		 */
 		if (to_tail)
-			to_tail->next = i;
+			to_tail->next = this;
 		else
-			*to = i;
-		to_tail = i;
-		i->next = NULL;
-		fr_pair_steal(ctx, i);
+			*to = this;
+		to_tail = this;
+		this->next = NULL;
+
+		if (move) {
+			fr_pair_steal(ctx, i);
+		} else {
+			talloc_free(i);
+		}
 	}
 }
 
+
+/** Move matching pairs between VALUE_PAIR lists
+ *
+ * Move pairs of a matching attribute number, vendor number and tag from the
+ * the input list to the output list.
+ *
+ * @note pairs which are moved have their parent changed to ctx.
+ *
+ * @note fr_pair_list_free should be called on the head of the old list to free unmoved
+	 attributes (if they're no longer needed).
+ *
+ * @param[in] ctx for talloc
+ * @param[in,out] to destination list.
+ * @param[in,out] from source list.
+ * @param[in] attr to match. If attribute PW_VENDOR_SPECIFIC and vendor 0,
+ *	will match (and therefore copy) only VSAs.
+ *	If attribute 0 and vendor 0  will match (and therefore copy) all
+ *	attributes.
+ * @param[in] vendor to match.
+ */
+void fr_pair_list_move_by_num(TALLOC_CTX *ctx, VALUE_PAIR **to, VALUE_PAIR **from,
+			      unsigned int attr, unsigned int vendor, int8_t tag)
+{
+	fr_pair_list_move_by_num_internal(ctx, to, from, attr, vendor, tag, true);
+}
+
+
+/** Copy / delete matching pairs between VALUE_PAIR lists
+ *
+ * Move pairs of a matching attribute number, vendor number and tag from the
+ * the input list to the output list.  Like fr_pair_list_move_by_num(), but
+ * instead does copy / delete.
+ *
+ * @note The pair is NOT reparented.  It is copied and deleted.
+ *
+ * @note fr_pair_list_free should be called on the head of the old list to free unmoved
+	 attributes (if they're no longer needed).
+ *
+ * @param[in] ctx for talloc
+ * @param[in,out] to destination list.
+ * @param[in,out] from source list.
+ * @param[in] attr to match. If attribute PW_VENDOR_SPECIFIC and vendor 0,
+ *	will match (and therefore copy) only VSAs.
+ *	If attribute 0 and vendor 0  will match (and therefore copy) all
+ *	attributes.
+ * @param[in] vendor to match.
+ * @param[in] tag to match, TAG_ANY matches any tag, TAG_NONE matches tagless VPs.
+ */
+void fr_pair_list_mcopy_by_num(TALLOC_CTX *ctx, VALUE_PAIR **to, VALUE_PAIR **from,
+			      unsigned int attr, unsigned int vendor, int8_t tag)
+{
+	fr_pair_list_move_by_num_internal(ctx, to, from, attr, vendor, tag, false);
+}
+
+
 /** Convert string value to native attribute value
  *
  * @param vp to assign value to.