Try to shift up to sbuff start as current sbuff can be child sbuff with no used space...

* Try to shift up to sbuff start as current sbuff can be child sbuff with no used space.

Co-authored-by: Arran Cudbard-Bell <a.cudbardb@freeradius.org>

diff --git a/src/lib/util/sbuff.c b/src/lib/util/sbuff.c
index 223db6206e54e930fef87f79caea3bc133de380c..bd2a2e3ba48afd1e0244882e44bccf09ebd23c2e 100644 (file)
--- a/src/lib/util/sbuff.c
+++ b/src/lib/util/sbuff.c
@@ -257,7 +257,7 @@ size_t fr_sbuff_shift(fr_sbuff_t *sbuff, size_t shift)
 size_t fr_sbuff_extend_file(fr_sbuff_t *sbuff, size_t extension)
 {
        fr_sbuff_t              *sbuff_i;
-       size_t                  read, available, total_read;
+       size_t                  read, available, total_read, shift;
        fr_sbuff_uctx_file_t    *fctx;
 
        CHECK_SBUFF_INIT(sbuff);
@@ -273,14 +273,29 @@ size_t fr_sbuff_extend_file(fr_sbuff_t *sbuff, size_t extension)
                return 0;       /* There's no way we could satisfy the extension request */
        }
 
-       if (fr_sbuff_used(sbuff)) {
+       /*
+        *      Shift out the maximum number of bytes we can
+        *      irrespective of the amount that was requested
+        *      as the extension.  It's more efficient to do
+        *      this than lots of small shifts, and just 
+        *      looking and the number of bytes used in the
+        *      deepest sbuff, and using that as the shift
+        *      amount, might mean we don't shift anything at
+        *      all!
+        *
+        *      fr_sbuff_shift will cap the max shift amount,
+        *      so markers and positions will remain valid for 
+        *      all sbuffs in the chain.
+        */
+       shift = fr_sbuff_current(sbuff) - fr_sbuff_buff(sbuff);
+       if (shift) {
                /*
                 *      Try and shift as much as we can out
                 *      of the buffer to make space.
                 *
                 *      Note: p and markers are constraints here.
                 */
-               fctx->shifted += fr_sbuff_shift(sbuff, fr_sbuff_used(sbuff));
+               fctx->shifted += fr_sbuff_shift(sbuff, shift);
        }
 
        available = fctx->buff_end - sbuff->end;

diff --git a/src/lib/util/sbuff_tests.c b/src/lib/util/sbuff_tests.c
index 0d56977453584a4d3e54969d5e54c370c4793bdd..be77e4af2d9235e65a19018c06219bf243b287b7 100644 (file)
--- a/src/lib/util/sbuff_tests.c
+++ b/src/lib/util/sbuff_tests.c
@@ -1019,26 +1019,39 @@ static void test_talloc_extend_with_marker(void)
 static void test_file_extend(void)
 {
        fr_sbuff_t      sbuff;
-       fr_sbuff_t      our_sbuff;
+       fr_sbuff_t      our_sbuff, child_sbuff;
        fr_sbuff_uctx_file_t    fctx;
        FILE            *fp;
-       char            buff[16];
+       char            buff[5];
        char            out[16 + 1];
-       char            fbuff[] = "                        xyzzy";
+       char            fbuff[24];
+       const char      PATTERN[] = "xyzzy";
+#define PATTERN_LEN (sizeof(PATTERN) - 1)
        char            *post_ws;
        ssize_t         slen;
 
+       static_assert(sizeof(buff) >= PATTERN_LEN, "Buffer must be sufficiently large to hold the pattern");
+       static_assert((sizeof(fbuff) % sizeof(buff)) > 0, "sizeof buff must not be a multiple of fbuff");
+       static_assert((sizeof(fbuff) % sizeof(buff)) < PATTERN_LEN, "remainder of sizeof(fbuff)/sizeof(buff) must be less than sizeof pattern");
+
        TEST_CASE("Initialization");
-       fp = fmemopen(fbuff, sizeof(fbuff) - 1, "r");
+       memset(fbuff, ' ', sizeof(fbuff));
+       memcpy(fbuff + sizeof(fbuff) - PATTERN_LEN, PATTERN, PATTERN_LEN);
+
+       fp = fmemopen(fbuff, sizeof(fbuff), "r");
        TEST_CHECK(fp != NULL);
        TEST_CHECK(fr_sbuff_init_file(&sbuff, &fctx, buff, sizeof(buff), fp, 128) == &sbuff);
        our_sbuff = FR_SBUFF_BIND_CURRENT(&sbuff);
 
        TEST_CASE("Advance past whitespace, which will require shift/extend");
-       TEST_CHECK_LEN(fr_sbuff_adv_past_whitespace(&our_sbuff, SIZE_MAX, NULL), sizeof(fbuff) - 6);
+       TEST_CHECK_LEN(fr_sbuff_adv_past_whitespace(&our_sbuff, SIZE_MAX, NULL), sizeof(fbuff) - PATTERN_LEN);
+       TEST_CASE("Verify extend on unused child buffer");
+       child_sbuff = FR_SBUFF(&our_sbuff);
+       slen = fr_sbuff_extend_file(&child_sbuff, 0);
+       TEST_CHECK_SLEN(slen, sizeof(fbuff) % PATTERN_LEN);
        TEST_CASE("Verify that we passed all and only whitespace");
        (void) fr_sbuff_out_abstrncpy(NULL, &post_ws, &our_sbuff, 24);
-       TEST_CHECK_STRCMP(post_ws, "xyzzy");
+       TEST_CHECK_STRCMP(post_ws, PATTERN);
        talloc_free(post_ws);
        TEST_CASE("Verify parent buffer end");
        TEST_CHECK(sbuff.end == our_sbuff.end);