laforge has submitted this change. ( 
https://gerrit.osmocom.org/c/libosmocore/+/35207?usp=email )

Change subject: util: add osmo_strbuf macros to manipulate the strbuf tail
......................................................................

util: add osmo_strbuf macros to manipulate the strbuf tail

Upcoming patch adopts osmo_strbuf in logging.c, which sometimes needs to
steal and re-add trailing newline characters, and also needs to let
ctime_r() write to the buffer before updating the osmo_strbuf state.

Related: OS#6284
Related: Ib577a5e0d7450ce93ff21f37ba3262704cbf4752
Change-Id: I997707c328eab3ffa00a78fdb9a0a2cbe18404b4
---
M include/osmocom/core/utils.h
M src/core/libosmocore.map
M src/core/utils.c
M tests/utils/utils_test.c
M tests/utils/utils_test.ok
5 files changed, 178 insertions(+), 0 deletions(-)

Approvals:
  pespin: Looks good to me, but someone else must approve
  osmith: Looks good to me, but someone else must approve
  laforge: Looks good to me, approved
  Jenkins Builder: Verified




diff --git a/include/osmocom/core/utils.h b/include/osmocom/core/utils.h
index 2a3670b..b6e67e2 100644
--- a/include/osmocom/core/utils.h
+++ b/include/osmocom/core/utils.h
@@ -284,6 +284,12 @@
 /*! Return remaining space for characters and terminating nul in the given 
struct osmo_strbuf. */
 #define OSMO_STRBUF_REMAIN(STRBUF) ((STRBUF).buf ? (STRBUF).len - 
((STRBUF).pos - (STRBUF).buf) : 0)

+/*! Return number of actual characters contained in struct osmo_strbuf 
(without terminating nul). */
+#define OSMO_STRBUF_CHAR_COUNT(STRBUF) ((STRBUF).buf && ((STRBUF).pos > 
(STRBUF).buf) ? \
+                                       OSMO_MIN((STRBUF).pos - (STRBUF).buf, \
+                                                (STRBUF).len - 1) \
+                                       : 0)
+
 /*! Like OSMO_STRBUF_APPEND(), but for function signatures that return the 
char* buffer instead of a length.
  * When using this function, the final STRBUF.chars_needed may not reflect the 
actual number of characters needed, since
  * that number cannot be obtained from this kind of function signature.
@@ -307,6 +313,16 @@
                (STRBUF).chars_needed += _sb_l; \
        } while(0)

+void osmo_strbuf_drop_tail(struct osmo_strbuf *sb, size_t n_chars);
+/* Convenience macro. struct osmo_strbuf are typically static to a function 
scope. Avoid having to type '&', same as
+ * with all the other OSMO_STRBUF_* API. */
+#define OSMO_STRBUF_DROP_TAIL(STRBUF, N_CHARS) 
osmo_strbuf_drop_tail(&(STRBUF), N_CHARS)
+
+void osmo_strbuf_added_tail(struct osmo_strbuf *sb, size_t n_chars);
+/* Convenience macro. struct osmo_strbuf are typically static to a function 
scope. Avoid having to type '&', same as
+ * with all the other OSMO_STRBUF_* API. */
+#define OSMO_STRBUF_ADDED_TAIL(STRBUF, N_CHARS) 
osmo_strbuf_added_tail(&(STRBUF), N_CHARS)
+
 bool osmo_str_startswith(const char *str, const char *startswith_str);

 int osmo_float_str_to_int(int64_t *val, const char *str, unsigned int 
precision);
diff --git a/src/core/libosmocore.map b/src/core/libosmocore.map
index fc81650..30c4927 100644
--- a/src/core/libosmocore.map
+++ b/src/core/libosmocore.map
@@ -510,6 +510,8 @@
 osmo_strrb_get_nth;
 _osmo_strrb_is_bufindex_valid;
 osmo_strrb_is_empty;
+osmo_strbuf_drop_tail;
+osmo_strbuf_added_tail;
 osmo_str_startswith;
 osmo_str_to_int;
 osmo_str_to_int64;
diff --git a/src/core/utils.c b/src/core/utils.c
index 231b65c..882eb6f 100644
--- a/src/core/utils.c
+++ b/src/core/utils.c
@@ -1211,6 +1211,51 @@
        return (sum * 9) % 10 + '0';
 }

+/*! Remove up to N chars from the end of an osmo_strbuf.
+ * |--char-count---| - - chars_needed - - |
+ *               |<---------drop----------|
+ */
+void osmo_strbuf_drop_tail(struct osmo_strbuf *sb, size_t n_chars)
+{
+       size_t drop_n;
+       if (sb->pos <= sb->buf)
+               return;
+       drop_n = OSMO_MIN(sb->chars_needed, n_chars);
+       sb->chars_needed -= drop_n;
+       /* chars_needed was reduced by n_chars, which may have been entirely 
behind the end of a full buffer, within the
+        * hypothetical chars_needed. Modify the buffer tail pos only if the 
buffer is not or longer full now. */
+       if (sb->chars_needed >= OSMO_STRBUF_CHAR_COUNT(*sb))
+               return;
+       sb->pos = sb->buf + sb->chars_needed;
+       *sb->pos = '\0';
+}
+
+/*! Let osmo_strbuf know that n_chars characters (excluding nul) were written 
to the end of the buffer.
+ * If sb is nonempty, the n_chars are assumed to have been written to sb->pos. 
If sb is still empty and pos == NULL, the
+ * n_chars are assumed to have been written to the start of the buffer.
+ * Advance sb->pos and sb->chars_needed by at most n_chars, or up to sb->len - 
1.
+ * Ensure nul termination. */
+void osmo_strbuf_added_tail(struct osmo_strbuf *sb, size_t n_chars)
+{
+       /* On init of an osmo_strbuf, sb->pos == NULL, which is defined as 
semantically identical to pointing at the
+        * start of the buffer. A caller may just write to the buffer and call 
osmo_strbuf_added_tail(), in which case
+        * still pos == NULL. pos != NULL happens as soon as the first 
OSMO_STRBUF_*() API has acted on the strbuf. */
+       if (!sb->pos)
+               sb->pos = sb->buf;
+       sb->chars_needed += n_chars;
+       /* first get remaining space, not counting trailing nul; but safeguard 
against empty buffer */
+       size_t n_added = OSMO_STRBUF_REMAIN(*sb);
+       if (n_added)
+               n_added--;
+       /* do not add more than fit in sb->len, still ensuring nul termination 
*/
+       n_added = OSMO_MIN(n_added, n_chars);
+       if (n_added)
+               sb->pos += n_added;
+       /* when a strbuf is full, sb->pos may point after the final nul, so nul 
terminate only when pos is valid. */
+       if (sb->pos < sb->buf + sb->len)
+               *sb->pos = '\0';
+}
+
 /*! Compare start of a string.
  * This is an optimisation of 'strstr(str, startswith_str) == str' because it 
doesn't search through the entire string.
  * \param str  (Longer) string to compare.
diff --git a/tests/utils/utils_test.c b/tests/utils/utils_test.c
index 0b7bfe4..bdeedb5 100644
--- a/tests/utils/utils_test.c
+++ b/tests/utils/utils_test.c
@@ -31,6 +31,7 @@
 #include <errno.h>
 #include <limits.h>
 #include <inttypes.h>
+#include <string.h>

 static void hexdump_test(void)
 {
@@ -1306,6 +1307,50 @@
        printf("%zu: %s (need=%zu)\n", sb.len, buf, sb.chars_needed);
 }

+void strbuf_test_tail_for_buflen(size_t buflen)
+{
+       char buf[buflen];
+       struct osmo_strbuf sb = { .buf = buf, .len = buflen };
+       printf("\n%s(%zu)\n", __func__, buflen);
+
+#define SHOW(N) \
+       printf(#N ": %s sb.chars_needed=%zu sb.pos=&sb.buf[%d]\n", \
+              osmo_quote_str(buf, -1), sb.chars_needed, (int)(sb.pos - sb.buf))
+
+       /* shorten in steps using OSMO_STRBUF_DROP_TAIL(), removing and 
re-adding a trailing newline. */
+       OSMO_STRBUF_PRINTF(sb, "banananana\n");
+       SHOW(1);
+       OSMO_STRBUF_DROP_TAIL(sb, 3);
+       SHOW(2);
+       OSMO_STRBUF_PRINTF(sb, "\n");
+       SHOW(3);
+       OSMO_STRBUF_DROP_TAIL(sb, 3);
+       SHOW(4);
+       OSMO_STRBUF_PRINTF(sb, "\n");
+       SHOW(5);
+
+       /* drop trailing newline */
+       OSMO_STRBUF_DROP_TAIL(sb, 1);
+       SHOW(6);
+
+       /* test writing something to the end and letting 
OSMO_STRBUF_ADDED_TAIL() know later */
+       int n = OSMO_MIN(6, OSMO_STRBUF_REMAIN(sb));
+       if (n)
+               memcpy(sb.pos, "bread\n", n);
+       OSMO_STRBUF_ADDED_TAIL(sb, 6);
+       SHOW(7);
+}
+
+void strbuf_test_tail(void)
+{
+       strbuf_test_tail_for_buflen(64);
+       strbuf_test_tail_for_buflen(32);
+       strbuf_test_tail_for_buflen(16);
+       strbuf_test_tail_for_buflen(8);
+       strbuf_test_tail_for_buflen(4);
+       strbuf_test_tail_for_buflen(1);
+}
+
 static void startswith_test_str(const char *str, const char *startswith_str, 
bool expect_rc)
 {
        bool rc = osmo_str_startswith(str, startswith_str);
@@ -2152,6 +2197,7 @@
        osmo_str_tolowupper_test();
        strbuf_test();
        strbuf_test_nolen();
+       strbuf_test_tail();
        startswith_test();
        name_c_impl_test();
        osmo_print_n_test();
diff --git a/tests/utils/utils_test.ok b/tests/utils/utils_test.ok
index 3f453e9..6f5d46b 100644
--- a/tests/utils/utils_test.ok
+++ b/tests/utils/utils_test.ok
@@ -470,6 +470,60 @@
 more: 0001011100101010000 (need=19)
 10: 000101110 (need=9)

+strbuf_test_tail_for_buflen(64)
+1: "banananana\n" sb.chars_needed=11 sb.pos=&sb.buf[11]
+2: "bananana" sb.chars_needed=8 sb.pos=&sb.buf[8]
+3: "bananana\n" sb.chars_needed=9 sb.pos=&sb.buf[9]
+4: "banana" sb.chars_needed=6 sb.pos=&sb.buf[6]
+5: "banana\n" sb.chars_needed=7 sb.pos=&sb.buf[7]
+6: "banana" sb.chars_needed=6 sb.pos=&sb.buf[6]
+7: "bananabread\n" sb.chars_needed=12 sb.pos=&sb.buf[12]
+
+strbuf_test_tail_for_buflen(32)
+1: "banananana\n" sb.chars_needed=11 sb.pos=&sb.buf[11]
+2: "bananana" sb.chars_needed=8 sb.pos=&sb.buf[8]
+3: "bananana\n" sb.chars_needed=9 sb.pos=&sb.buf[9]
+4: "banana" sb.chars_needed=6 sb.pos=&sb.buf[6]
+5: "banana\n" sb.chars_needed=7 sb.pos=&sb.buf[7]
+6: "banana" sb.chars_needed=6 sb.pos=&sb.buf[6]
+7: "bananabread\n" sb.chars_needed=12 sb.pos=&sb.buf[12]
+
+strbuf_test_tail_for_buflen(16)
+1: "banananana\n" sb.chars_needed=11 sb.pos=&sb.buf[11]
+2: "bananana" sb.chars_needed=8 sb.pos=&sb.buf[8]
+3: "bananana\n" sb.chars_needed=9 sb.pos=&sb.buf[9]
+4: "banana" sb.chars_needed=6 sb.pos=&sb.buf[6]
+5: "banana\n" sb.chars_needed=7 sb.pos=&sb.buf[7]
+6: "banana" sb.chars_needed=6 sb.pos=&sb.buf[6]
+7: "bananabread\n" sb.chars_needed=12 sb.pos=&sb.buf[12]
+
+strbuf_test_tail_for_buflen(8)
+1: "bananan" sb.chars_needed=11 sb.pos=&sb.buf[8]
+2: "bananan" sb.chars_needed=8 sb.pos=&sb.buf[8]
+3: "bananan" sb.chars_needed=9 sb.pos=&sb.buf[8]
+4: "banana" sb.chars_needed=6 sb.pos=&sb.buf[6]
+5: "banana\n" sb.chars_needed=7 sb.pos=&sb.buf[7]
+6: "banana" sb.chars_needed=6 sb.pos=&sb.buf[6]
+7: "bananab" sb.chars_needed=12 sb.pos=&sb.buf[7]
+
+strbuf_test_tail_for_buflen(4)
+1: "ban" sb.chars_needed=11 sb.pos=&sb.buf[4]
+2: "ban" sb.chars_needed=8 sb.pos=&sb.buf[4]
+3: "ban" sb.chars_needed=9 sb.pos=&sb.buf[4]
+4: "ban" sb.chars_needed=6 sb.pos=&sb.buf[4]
+5: "ban" sb.chars_needed=7 sb.pos=&sb.buf[4]
+6: "ban" sb.chars_needed=6 sb.pos=&sb.buf[4]
+7: "ban" sb.chars_needed=12 sb.pos=&sb.buf[4]
+
+strbuf_test_tail_for_buflen(1)
+1: "" sb.chars_needed=11 sb.pos=&sb.buf[1]
+2: "" sb.chars_needed=8 sb.pos=&sb.buf[1]
+3: "" sb.chars_needed=9 sb.pos=&sb.buf[1]
+4: "" sb.chars_needed=6 sb.pos=&sb.buf[1]
+5: "" sb.chars_needed=7 sb.pos=&sb.buf[1]
+6: "" sb.chars_needed=6 sb.pos=&sb.buf[1]
+7: "" sb.chars_needed=12 sb.pos=&sb.buf[1]
+
 startswith_test()
 osmo_str_startswith(NULL, NULL) == true
 osmo_str_startswith("", NULL) == true

--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35207?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I997707c328eab3ffa00a78fdb9a0a2cbe18404b4
Gerrit-Change-Number: 35207
Gerrit-PatchSet: 3
Gerrit-Owner: neels <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-Reviewer: osmith <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>
Gerrit-MessageType: merged

Reply via email to