From: Arran Cudbard-Bell <a.cudbardb@freeradius.org>
Date: Thu, 10 Jun 2021 20:20:53 +0000 (-0500)
Subject: Sync up dbuff/sbuff shift and update
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=284179d412752a0aad9052a2767459688cdcf76d;p=thirdparty%2Ffreeradius-server.git

Sync up dbuff/sbuff shift and update

Make shift more efficient by avoiding memmove if the entire contents of the buffer is shifted.

Fix sbuff shift so it's not broken.
---

diff --git a/src/lib/util/dbuff.c b/src/lib/util/dbuff.c
index 10fc4601991..5c9e1afc4da 100644
--- a/src/lib/util/dbuff.c
+++ b/src/lib/util/dbuff.c
@@ -115,11 +115,14 @@ void fr_dbuff_update(fr_dbuff_t *dbuff, uint8_t *new_buff, size_t new_len)
  */
 size_t fr_dbuff_shift(fr_dbuff_t *dbuff, size_t shift)
 {
-	uint8_t	*buff;		/* Current start */
+	uint8_t		*buff, *end;		/* Current start */
+	fr_dbuff_t	*dbuff_i;
+	size_t		max_shift = shift;
 
 	CHECK_DBUFF_INIT(dbuff);
 
 	buff = dbuff->buff;
+	end = dbuff->end;
 
 	/*
 	 *	First pass: find the maximum shift, which is the minimum
@@ -127,12 +130,17 @@ size_t fr_dbuff_shift(fr_dbuff_t *dbuff, size_t shift)
 	 *	or current pointers of markers of dbuff and its ancestors.
 	 *	(We're also constrained by the requested shift count.)
 	 */
-	for (fr_dbuff_t *d = dbuff; d && shift; d = d->parent) {
-		shift = min(shift, d->p - buff);
-		for (fr_dbuff_marker_t *m = d->m; m; m = m->next) shift = min(shift, m->p - buff);
-	}
+	for (dbuff_i = dbuff; dbuff_i; dbuff_i = dbuff_i->parent) {
+		fr_dbuff_marker_t *m_i;
 
-	if (!shift) return 0;
+		max_shift = min(max_shift, dbuff_i->p - buff);
+		if (!max_shift) return 0;
+
+		for (m_i = dbuff_i->m; m_i; m_i = m_i->next) {
+			max_shift = min(max_shift, m_i->p - buff);
+			if (!max_shift) return 0;
+		}
+	}
 
 	/*
 	 *	Second pass: adjust pointers.
@@ -142,22 +150,23 @@ size_t fr_dbuff_shift(fr_dbuff_t *dbuff, size_t shift)
 	 *	For d->end, d->buff <= d->p - shift, and d->p shouldn't exceed d->end,
 	 *	so d->p - shift <= d->end - shift, hence d->buff <= d->end - shift.
 	 */
-	for (fr_dbuff_t *d = dbuff; d; d = d->parent) {
-		uint8_t	*start = d->start;
-
-		d->start -= min(shift, d->start - buff);
-		d->p -= shift;
-		d->end -= shift;
-		d->shifted += (shift - (start - d->start));
-		for (fr_dbuff_marker_t *m = d->m; m; m = m->next) m->p -= shift;
+	for (dbuff_i = dbuff; dbuff_i; dbuff_i = dbuff_i->parent) {
+		fr_dbuff_marker_t	*m_i;
+		uint8_t			*start = dbuff_i->start;
+
+		dbuff_i->start -= min(max_shift, dbuff_i->start - buff);
+		dbuff_i->p -= max_shift;
+		dbuff_i->end -= max_shift;
+		dbuff_i->shifted += (max_shift - (start - dbuff_i->start));
+		for (m_i = dbuff_i->m; m_i; m_i = m_i->next) m_i->p -= max_shift;
 	}
 
 	/*
 	 *	Move data to track moved pointers.
 	 */
-	memmove(buff, buff + shift, ((fr_dbuff_uctx_fd_t *)(dbuff->uctx))->buff_end - (buff + shift));
+	if ((buff + max_shift) < end) memmove(buff, buff + max_shift, end - (buff + max_shift));
 
-	return shift;
+	return max_shift;
 }
 
 /** Refresh the buffer with more data from the file
diff --git a/src/lib/util/sbuff.c b/src/lib/util/sbuff.c
index 1ebd1f99bee..e2438405068 100644
--- a/src/lib/util/sbuff.c
+++ b/src/lib/util/sbuff.c
@@ -114,6 +114,11 @@ static inline CC_HINT(always_inline) ssize_t safecpy(char *o_start, char *o_end,
 	return (i_len);
 }
 
+static inline CC_HINT(always_inline) size_t min(size_t x, size_t y)
+{
+	return x < y ? x : y;
+}
+
 /** Update all markers and pointers in the set of sbuffs to point to new_buff
  *
  * This function should be used if the underlying buffer is realloced.
@@ -127,30 +132,26 @@ void fr_sbuff_update(fr_sbuff_t *sbuff, char *new_buff, size_t new_len)
 	fr_sbuff_t		*sbuff_i;
 	char			*old_buff;	/* Current buff */
 
-#define update_ptr(_old_buff, _new_buff, _new_len, _field) \
-	_field = (size_t)((_field) - (_old_buff)) < _new_len ? \
-		(_new_buff) + ((_field) - (_old_buff)) : \
-		(_new_buff) + (_new_len)
-
 	old_buff = sbuff->buff;
 
 	/*
 	 *	Update pointers to point to positions
 	 *	in new buffer based on their relative
-	 *	offsets in the old buffer.
+	 *	offsets in the old buffer... but not
+	 *	past the end of the new buffer.
 	 */
 	for (sbuff_i = sbuff; sbuff_i; sbuff_i = sbuff_i->parent) {
 		fr_sbuff_marker_t	*m_i;
 
 		sbuff_i->buff = new_buff;
-		update_ptr(old_buff, new_buff, new_len, sbuff_i->start);
+		sbuff_i->start = new_buff + min(new_len, sbuff_i->start - old_buff);
 		sbuff_i->end = sbuff_i->buff + new_len;
 		*(sbuff_i->end) = '\0';	/* Re-terminate */
-		update_ptr(old_buff, new_buff, new_len, sbuff_i->p);
 
-		for (m_i = sbuff_i->m; m_i; m_i = m_i->next) update_ptr(old_buff, new_buff, new_len, m_i->p);
+		sbuff_i->p = new_buff + min(new_len, sbuff_i->p - old_buff);
+
+		for (m_i = sbuff_i->m; m_i; m_i = m_i->next) m_i->p = new_buff + min(new_len, m_i->p - old_buff);
 	}
-#undef update_ptr
 }
 
 /** Shift the contents of the sbuff, returning the number of bytes we managed to shift
@@ -168,16 +169,14 @@ void fr_sbuff_update(fr_sbuff_t *sbuff, char *new_buff, size_t new_len)
 size_t fr_sbuff_shift(fr_sbuff_t *sbuff, size_t shift)
 {
 	fr_sbuff_t		*sbuff_i;
-	char			*buff;		/* Current start */
+	char			*buff, *end;		/* Current start */
 	size_t			max_shift = shift;
 	bool			reterminate = false;
 
 	CHECK_SBUFF_INIT(sbuff);
 
-#define update_ptr(_buff, _shift, _field) _field = (size_t)((_field) - (_buff)) < (_shift) ? (_buff) : ((_field) - (_shift))
-#define update_max_shift(_buff, _max_shift, _field) if (((_buff) + (_max_shift)) > (_field)) _max_shift -= (((_buff) + (_max_shift)) - (_field))
-
 	buff = sbuff->buff;
+	end = sbuff->end;
 
 	/*
 	 *	If the sbuff is already \0 terminated
@@ -188,45 +187,49 @@ size_t fr_sbuff_shift(fr_sbuff_t *sbuff, size_t shift)
 	reterminate = (sbuff->p < sbuff->end) && (*sbuff->p == '\0') && !sbuff->is_const;
 
 	/*
-	 *	Determine the maximum shift amount.
-	 *	Shifts are constrained by the position
-	 *	of pointers into the buffer.
+	 *	First pass: find the maximum shift, which is the minimum
+	 *	of the distances from buff to any of the current pointers
+	 *	or current pointers of markers of dbuff and its ancestors.
+	 *	(We're also constrained by the requested shift count.)
 	 */
 	for (sbuff_i = sbuff; sbuff_i; sbuff_i = sbuff_i->parent) {
-		fr_sbuff_marker_t	*m_i;
+		fr_sbuff_marker_t *m_i;
 
-		update_max_shift(buff, max_shift, sbuff_i->p);
+		max_shift = min(max_shift, sbuff_i->p - buff);
 		if (!max_shift) return 0;
 
 		for (m_i = sbuff_i->m; m_i; m_i = m_i->next) {
-			update_max_shift(buff, max_shift, m_i->p);
+			max_shift = min(max_shift, m_i->p - buff);
 			if (!max_shift) return 0;
 		}
 	}
 
+	/*
+	 *	Second pass: adjust pointers.
+	 *	The first pass means we need only subtract shift from
+	 *	current pointers.  Start pointers can't constrain shift,
+	 *	or we'd never free any space, so they require the added
+	 *	check.
+	 */
 	for (sbuff_i = sbuff; sbuff_i; sbuff_i = sbuff_i->parent) {
 		fr_sbuff_marker_t	*m_i;
 		char			*start = sbuff_i->start;
 
-		/*
-		 *	Current position shifts, but stays the same
-		 *	relative to content.
-		 */
-		update_ptr(buff, max_shift, sbuff_i->start);
-		update_ptr(buff, max_shift, sbuff_i->p);
-		update_ptr(buff, max_shift, sbuff_i->end);
+		sbuff_i->start -= min(max_shift, sbuff_i->start - buff);
+		sbuff_i->p -= max_shift;
+		sbuff_i->end -= max_shift;
 		sbuff_i->shifted += (max_shift - (start - sbuff_i->start));
-
-		for (m_i = sbuff_i->m; m_i; m_i = m_i->next) update_ptr(buff, max_shift, m_i->p);
+		for (m_i = sbuff_i->m; m_i; m_i = m_i->next) m_i->p -= max_shift;
 	}
 
-//	memmove(sbuff->buff, sbuff->buff + max_shift, max_shift);
+	/*
+	 *	Only memmove if the shift wasn't the
+	 *      entire contents of the buffer.
+	 */
+	if ((buff + max_shift) < end) memmove(buff, buff + max_shift, end - (buff + max_shift));
 
 	if (reterminate) *sbuff->p = '\0';
 
-#undef update_ptr
-#undef update_max_shift
-
 	return max_shift;
 }
 
@@ -385,6 +388,37 @@ int fr_sbuff_trim_talloc(fr_sbuff_t *sbuff, size_t len)
 	return 0;
 }
 
+/** Reset a talloced buffer to its initial length, clearing any data stored
+ *
+ * @param[in] sbuff to reset.
+ * @return
+ *	- 0 on success.
+ *	- -1 on failure - markers present pointing past the end of string data.
+ */
+int fr_sbuff_reset_talloc(fr_sbuff_t *sbuff)
+{
+	fr_sbuff_uctx_talloc_t	*tctx = sbuff->uctx;
+
+	CHECK_SBUFF_INIT(sbuff);
+
+	fr_sbuff_set_to_start(sbuff);	/* Clear data */
+
+	if (fr_sbuff_used(sbuff) != tctx->init) {
+		char *new_buff;
+
+		new_buff = talloc_realloc(tctx->ctx, sbuff->buff, char, tctx->init);
+		if (!new_buff) {
+			fr_strerror_printf("Failed reallocing from %zu to %zu",
+					   talloc_array_length(sbuff->buff), tctx->init);
+			return -1;
+		}
+		sbuff->buff = new_buff;
+		fr_sbuff_update(sbuff, new_buff, tctx->init - 1);
+	}
+
+	return 0;
+}
+
 /** Fill as much of the output buffer we can and break on partial copy
  *
  * @param[in] _out	sbuff to write to.
diff --git a/src/lib/util/sbuff.h b/src/lib/util/sbuff.h
index d0b1b8dc3a8..d2b29d1412b 100644
--- a/src/lib/util/sbuff.h
+++ b/src/lib/util/sbuff.h
@@ -494,6 +494,8 @@ size_t	fr_sbuff_extend_talloc(fr_sbuff_t *sbuff, size_t extenison);
 
 int	fr_sbuff_trim_talloc(fr_sbuff_t *sbuff, size_t len);
 
+int	fr_sbuff_reset_talloc(fr_sbuff_t *sbuff);
+
 static inline void fr_sbuff_terminate(fr_sbuff_t *sbuff)
 {
 	*sbuff->p = '\0';