From: James Jones Date: Thu, 17 Dec 2020 00:00:12 +0000 (-0600) Subject: Add "extensible" dbuffs that read data from a file and relevant tests. (#3761) X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4b81ff62878dc623a7f9ca1e8f827bbd21cbc3a6;p=thirdparty%2Ffreeradius-server.git Add "extensible" dbuffs that read data from a file and relevant tests. (#3761) --- diff --git a/src/lib/util/dbuff.c b/src/lib/util/dbuff.c index b32c53536b3..cf32e154abe 100644 --- a/src/lib/util/dbuff.c +++ b/src/lib/util/dbuff.c @@ -26,6 +26,7 @@ RCSID("$Id$") #include +#include #if defined(__clang_analyzer__) || !defined(NDEBUG) # define CHECK_DBUFF_INIT(_sbuff) if (!(_sbuff)->extend && (unlikely(!(_sbuff)->buff) || unlikely(!(_sbuff)->start) || unlikely(!(_sbuff)->end) || unlikely(!(_sbuff)->p))) return 0; @@ -98,6 +99,117 @@ void fr_dbuff_update(fr_dbuff_t *dbuff, uint8_t *new_buff, size_t new_len) } } +/** Shift the contents of the sbuff, returning the number of bytes we managed to shift + * + * @param[in] dbuff to shift. + * @param[in] shift the number of bytes to shift + * (not necessarily the amount actually shifted, because + * one can't move pointers outside the buffer) + * + * @return + * - 0 the shift failed due to constraining pointers. + * - >0 the number of bytes we managed to shift pointers + * in the dbuff. + */ +size_t fr_dbuff_shift(fr_dbuff_t *dbuff, size_t shift) +{ + uint8_t *buff; /* Current start */ + + CHECK_DBUFF_INIT(dbuff); + + buff = dbuff->buff; + + /* + * 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 (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); + } + + if (!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 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; + } + + /* + * Move data to track moved pointers. + */ + memmove(buff, buff + shift, ((fr_dbuff_uctx_fd_t *)(dbuff->uctx))->buff_end - (buff + shift)); + + return shift; +} + +/** Refresh the buffer with more data from the file + * + */ +size_t _fr_dbuff_extend_fd(fr_dbuff_t *dbuff, size_t extension) +{ + ssize_t bytes_read; + size_t available, total_read; + fr_dbuff_uctx_fd_t *fctx; + + CHECK_DBUFF_INIT(dbuff); + + fctx = dbuff->uctx; + + if (extension == SIZE_MAX) extension = 0; + + total_read = dbuff->shifted + (dbuff->end - dbuff->buff); + if (total_read >= fctx->max) { + fr_strerror_printf("Can't satisfy extension request for %zu bytes", extension); + return 0; /* There's no way we could satisfy the extension request */ + } + + if (fr_dbuff_used(dbuff)) { + /* + * Try and shift as much as we can out + * of the buffer to make space. + * + * Note: p and markers are constraints here. + */ + fr_dbuff_shift(dbuff, fr_dbuff_used(dbuff)); + } + + available = min(fctx->buff_end - dbuff->end, fctx->max - total_read); + if (available < extension) { + fr_strerror_printf("Can't satisfy extension request for %zu bytes", extension); + return 0; /* There's no way we could satisfy the extension request */ + } + + bytes_read = read(fctx->fd, dbuff->end, available); + + /** Check for errors + */ + if (bytes_read < 0) { + fr_strerror_printf("Error extending buffer: %s", fr_syserror(errno)); + return 0; + } + + /* Question: should extensible ancestors have their ends adjusted? */ + dbuff->end += bytes_read; /* Advance end, which increases fr_dbuff_remaining() */ + + return bytes_read; +} + /** Reallocate the current buffer * * @private diff --git a/src/lib/util/dbuff.h b/src/lib/util/dbuff.h index b0511fcd674..67e408a713f 100644 --- a/src/lib/util/dbuff.h +++ b/src/lib/util/dbuff.h @@ -157,6 +157,9 @@ struct fr_dbuff_s { uint8_t adv_parent:1; //!< Whether we advance the parent ///< of this dbuff. + size_t shifted; //!< How many bytes this sbuff has been + ///< shifted since its creation. + fr_dbuff_extend_t extend; //!< Function to re-populate or extend ///< the buffer. void *uctx; //!< Extend uctx data. @@ -408,6 +411,54 @@ static inline fr_dbuff_t *fr_dbuff_init_talloc(TALLOC_CTX *ctx, return dbuff; } +size_t _fr_dbuff_extend_fd(fr_dbuff_t *dbuff, size_t extension); + +/** File sbuff extension structure use by #fr_dbuff_init_fd + * @private + * + * Holds the data necessary for creating dynamically + * extensible file buffers. + */ +typedef struct { + int fd; //!< fd of file we're reading from. + uint8_t *buff_end; //!< The true end of the buffer. + size_t max; //!< Maximum number of bytes to read. +} fr_dbuff_uctx_fd_t; + + +/** Initialise a special dbuff which automatically reads in more data as the buffer is exhausted + * + * @param[out] dbuff to initialise. + * @param[out] fctx to initialise. Must have a lifetime >= to the dbuff. + * @param[in] buff Temporary buffer to use for storing file contents. + * @param[in] len Length of the temporary buffer. + * @param[in] fd descriptor of an open file to read from. + * @param[in] max The maximum length of data to read from the file. + * @return + * - The passed dbuff on success. + * - NULL on failure. + */ +static inline fr_dbuff_t *fr_dbuff_init_fd(fr_dbuff_t *dbuff, fr_dbuff_uctx_fd_t *fctx, + uint8_t *buff, size_t len, int fd, size_t max) +{ + *fctx = (fr_dbuff_uctx_fd_t){ + .fd = fd, + .max = max, + .buff_end = buff + len //!< Store the real end + }; + + *dbuff = (fr_dbuff_t){ + .buff = buff, + .start = buff, + .p = buff, + .end = buff, //!< Starts with 0 bytes available + .extend = _fr_dbuff_extend_fd, + .uctx = fctx + }; + + return dbuff; +} + /** Creates a compound literal to pass into functions which accept a dbuff * * @note The return value of the function should be used to determine how much @@ -490,7 +541,7 @@ typedef enum { static inline size_t _fr_dbuff_extend_lowat(fr_dbuff_extend_status_t *status, fr_dbuff_t *in, size_t remaining, size_t lowat) { - size_t extended; + size_t extended = 0; if (status && !fr_dbuff_is_extendable(*status)) { not_extendable: @@ -583,6 +634,8 @@ do { \ * @{ */ 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); /** @} */ /** @name Length checks @@ -1417,9 +1470,18 @@ size_t _fr_dbuff_move_dbuff_marker_to_dbuff_marker(fr_dbuff_marker_t *out, fr_db */ static inline ssize_t _fr_dbuff_out_memcpy(uint8_t *out, uint8_t **pos_p, fr_dbuff_t *in, size_t outlen) { - FR_DBUFF_EXTEND_LOWAT_OR_RETURN(in, outlen); + size_t ext_len, to_copy, remaining; + + for (remaining = outlen; remaining > 0; remaining -= to_copy) { + to_copy = remaining; + ext_len = _fr_dbuff_extend_lowat(NULL, in, fr_dbuff_end(in) - (*pos_p), 1); + if (ext_len == 0) return -remaining; + if (ext_len < to_copy) to_copy = ext_len; + out += _fr_dbuff_set(pos_p, in, + (*pos_p) + _fr_dbuff_safecpy(out, out + to_copy, (*pos_p), (*pos_p) + to_copy)); + } - return _fr_dbuff_set(pos_p, in, (*pos_p) + _fr_dbuff_safecpy(out, out + outlen, (*pos_p), (*pos_p) + outlen)); + return outlen; } /** Internal function - do not call directly * @@ -1427,16 +1489,9 @@ static inline ssize_t _fr_dbuff_out_memcpy(uint8_t *out, uint8_t **pos_p, fr_dbu */ static inline ssize_t _fr_dbuff_out_memcpy_dbuff(uint8_t **out_p, fr_dbuff_t *out, uint8_t **pos_p, fr_dbuff_t *in, size_t outlen) { - size_t ext_len; - - if (outlen == SIZE_MAX) { - ext_len = _fr_dbuff_extend_lowat(NULL, out, fr_dbuff_end(out) - (*out_p), outlen); - if (ext_len < outlen) outlen = ext_len; - } else { - _FR_DBUFF_EXTEND_LOWAT_POS_OR_RETURN(out_p, out, outlen); /* Extend out or return */ - } + if (outlen == SIZE_MAX) outlen = _fr_dbuff_extend_lowat(NULL, out, fr_dbuff_end(out) - (*out_p), outlen); - return _fr_dbuff_out_memcpy(*out_p, pos_p, in, outlen); + return _fr_dbuff_out_memcpy((*out_p), pos_p, in, outlen); } /** Copy exactly _outlen bytes from the dbuff diff --git a/src/lib/util/dbuff_tests.c b/src/lib/util/dbuff_tests.c index cffbed92cc8..a58b0af8ee3 100644 --- a/src/lib/util/dbuff_tests.c +++ b/src/lib/util/dbuff_tests.c @@ -1,5 +1,6 @@ #include #include +#include #include "dbuff.h" @@ -383,6 +384,68 @@ static void test_dbuff_talloc_extend_multi_level(void) TEST_CHECK(fr_dbuff_in(&dbuff2, (uint64_t) 0x123456789abcdef0) == -8); } +static void test_dbuff_fd(void) +{ + uint8_t const data[] = {0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef, + 0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef}; + int fd[2]; + fr_dbuff_t dbuff; + fr_dbuff_uctx_fd_t fctx; + uint8_t buff[4]; + uint8_t output[8]; + + /* + * buff is far smaller than one would ever use in practice; + * it's made that way to force multiple shift/extend operations. + */ + TEST_CASE("Initial allocation"); + TEST_CHECK(pipe(fd) == 0); + TEST_CHECK(write(fd[1], data, sizeof(data)) == sizeof(data)); + close(fd[1]); + TEST_CHECK(fr_dbuff_init_fd(&dbuff, &fctx, buff, sizeof(buff), fd[0], 24) == &dbuff); + TEST_CASE("Initial extend"); + TEST_CHECK(fr_dbuff_out_memcpy(output, &dbuff, 1) == 1); + TEST_CHECK(memcmp(output, data, 1) == 0); + TEST_CHECK(fr_dbuff_out_memcpy(output, &dbuff, 2) == 2); + TEST_CHECK(memcmp(output, &data[1], 2) == 0); + TEST_CASE("Leftover byte plus data from next extend"); + TEST_CHECK(fr_dbuff_out_memcpy(output, &dbuff, 4) == 4); + TEST_CHECK(memcmp(output, &data[3], 4) == 0); + TEST_CASE("Multiple extends"); + TEST_CHECK(fr_dbuff_out_memcpy(output, &dbuff, 8) == 8); + TEST_CHECK(memcmp(output, &data[7], 8) == 0); + TEST_CASE("EOF"); + TEST_CHECK(fr_dbuff_out_memcpy(output, &dbuff, 4) == -3); + close(fd[0]); +} + +static void test_dbuff_fd_max(void) +{ + uint8_t const data[] = {0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef, + 0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef}; + int fd[2]; + fr_dbuff_t dbuff; + fr_dbuff_uctx_fd_t fctx; + uint8_t buff[4]; + uint8_t output[8]; + + TEST_CASE("Initial allocation"); + TEST_CHECK(pipe(fd) == 0); + TEST_CHECK(write(fd[1], data, sizeof(data)) == sizeof(data)); + close(fd[1]); + TEST_CHECK(fr_dbuff_init_fd(&dbuff, &fctx, buff, sizeof(buff), fd[0], 8) == &dbuff); + TEST_CASE("Initial extend"); + TEST_CHECK(fr_dbuff_out_memcpy(output, &dbuff, 1) == 1); + TEST_CHECK(memcmp(output, data, 1) == 0); + TEST_CHECK(fr_dbuff_out_memcpy(output, &dbuff, 2) == 2); + TEST_CHECK(memcmp(output, &data[1], 2) == 0); + TEST_CHECK(fr_dbuff_out_memcpy(output, &dbuff, 4) == 4); + TEST_CHECK(memcmp(output, &data[3], 4) == 0); + TEST_CASE("Confirm that max precludes another shift/extend"); + TEST_CHECK(fr_dbuff_out_memcpy(output, &dbuff, 4) == -3); + close(fd[0]); +} + /** Test functions that read from dbuffs. * */ @@ -494,7 +557,9 @@ TEST_LIST = { { "fr_dbuff_move", test_dbuff_move }, { "fr_dbuff_talloc_extend", test_dbuff_talloc_extend }, { "fr_dbuff_talloc_extend_multi_level", test_dbuff_talloc_extend_multi_level }, - { "fr_dbff_out", test_dbuff_out }, + { "fr_dbuff_fd", test_dbuff_fd }, + { "fr_dbuff_fd_max", test_dbuff_fd_max }, + { "fr_dbuff_out", test_dbuff_out }, { NULL }