]> git.ipfire.org Git - thirdparty/collectd.git/commitdiff
strbuf: Add STRBUF_CREATE_FIXED, unify naming.
authorFlorian Forster <octo@google.com>
Fri, 19 Jun 2020 08:14:53 +0000 (10:14 +0200)
committerFlorian Forster <octo@google.com>
Tue, 14 Jul 2020 17:18:50 +0000 (19:18 +0200)
This changes the macro and function name to use "fixed" when a buffer and
its size are provided, and "static" if `sizeof()` is used to determine the
size.

The macros now return and expect a `strbuf_t`, i.e. *not* a pointer. This
makes it easier to see that the buffer is allocated on the stack and may
go out of scope.

The buffer size in the tests has been changed to 9, so that `sizeof(array)`
and `sizeof(pointer)` return different values.

src/utils/strbuf/strbuf.c
src/utils/strbuf/strbuf.h
src/utils/strbuf/strbuf_test.c

index f61f534ee485de743a94104901fd103d1671d9ee..80281715803dd63923cad4c8002520298dd50981 100644 (file)
@@ -95,7 +95,7 @@ static int strbuf_resize(strbuf_t *buf, size_t need) {
 
 strbuf_t *strbuf_create(void) { return calloc(1, sizeof(strbuf_t)); }
 
-strbuf_t *strbuf_create_static(void *buffer, size_t buffer_size) {
+strbuf_t *strbuf_create_fixed(void *buffer, size_t buffer_size) {
   strbuf_t *buf = calloc(1, sizeof(*buf));
   if (buf == NULL)
     return NULL;
index 29536dba5941b9ca15f71a5f161e538f160eb746..3bdde162f1ef1c6ceb9cfa8e591183c296058569 100644 (file)
@@ -38,32 +38,41 @@ typedef struct {
  * using STRBUF_DESTROY before returning from the function. Failure to call
  * STRBUF_DESTROY will leak the memory allocated to (strbuf_t).ptr. */
 #define STRBUF_CREATE                                                          \
-  &(strbuf_t) { .ptr = NULL }
+  (strbuf_t) { .ptr = NULL }
 
-/* STRBUF_CREATE_STATIC allocates a new strbuf_t on the stack, using the fixed
- * sized buffer "b". You may optionally call STRBUF_DESTROY with a fixed-sized
- * buffer. */
+/* STRBUF_CREATE_FIXED allocates a new strbuf_t on the stack, using the buffer
+ * "b" of fixed size "sz". The buffer is freed automatically when it goes out
+ * of scope. */
+#define STRBUF_CREATE_FIXED(b, sz)                                             \
+  (strbuf_t) { .ptr = b, .size = sz, .fixed = 1 }
+
+/* STRBUF_CREATE_STATIC allocates a new strbuf_t on the stack, using the static
+ * buffer "b". This macro assumes that is can use `sizeof(b)` to determine the
+ * size of "b". If that is not the case, use STRBUF_CREATE_FIXED instead. */
 #define STRBUF_CREATE_STATIC(b)                                                \
-  &(strbuf_t) { .ptr = b, .size = sizeof(b), .fixed = 1 }
+  (strbuf_t) { .ptr = b, .size = sizeof(b), .fixed = 1 }
 
 /* STRBUF_DESTROY frees the memory allocated inside the buffer. The buffer
- * itself is assumed to be allocated on the stack and is not freed. */
+ * itself is assumed to be allocated on the stack and is not freed. Calling
+ * STRBUF_DESTROY with a buffer that was allocated with STRBUF_CREATE_FIXED or
+ * STRBUF_CREATE_STATIC is a no-op. */
 #define STRBUF_DESTROY(buf)                                                    \
   do {                                                                         \
-    if (!buf->fixed) {                                                         \
-      free(buf->ptr);                                                          \
+    if (buf.fixed) {                                                           \
+      break;                                                                   \
     }                                                                          \
-    *buf = (strbuf_t){.ptr = NULL};                                            \
+    free(buf.ptr);                                                             \
+    buf.ptr = NULL;                                                            \
   } while (0)
 
 /* strbuf_create allocates a new strbuf_t on the heap, which must be freed
  * using strbuf_destroy. */
 strbuf_t *strbuf_create(void);
 
-/* strbuf_create_static allocates a new strbuf_t on the stack, using the fixed
+/* strbuf_create_fixed allocates a new strbuf_t on the stack, using the fixed
  * sized buffer "buffer". The returned strbuf_t* must be freed using
  * strbuf_destroy. */
-strbuf_t *strbuf_create_static(void *buffer, size_t buffer_size);
+strbuf_t *strbuf_create_fixed(void *buffer, size_t buffer_size);
 
 /* strbuf_destroy frees a strbuf_t* allocated on the heap. */
 void strbuf_destroy(strbuf_t *buf);
index 8323da3c8a23e2241af360ca66d4f3bb862a602d..b0ecf036b894be933af35c2e0ce9727dac5d4769 100644 (file)
@@ -30,7 +30,7 @@
 #include <errno.h>
 #include <stdbool.h>
 
-#define STATIC_BUFFER_SIZE 8
+#define STATIC_BUFFER_SIZE 9
 
 int test_buffer(strbuf_t *buf, bool is_static) {
   CHECK_ZERO(strbuf_print(buf, "foo"));
@@ -40,19 +40,19 @@ int test_buffer(strbuf_t *buf, bool is_static) {
   EXPECT_EQ_STR("foobar", buf->ptr);
 
   CHECK_ZERO(strbuf_printf(buf, "%d\n", 9000));
-  char const *want = is_static ? "foobar9" : "foobar9000\n";
+  char const *want = is_static ? "foobar90" : "foobar9000\n";
   EXPECT_EQ_STR(want, buf->ptr);
 
   if (is_static) {
     EXPECT_EQ_INT(ENOSPC, strbuf_print(buf, "buffer already filled"));
-    EXPECT_EQ_STR("foobar9", buf->ptr);
+    EXPECT_EQ_STR("foobar90", buf->ptr);
   }
 
   strbuf_reset(buf);
   CHECK_ZERO(strlen(buf->ptr));
 
   CHECK_ZERO(strbuf_print(buf, "new content"));
-  want = is_static ? "new con" : "new content";
+  want = is_static ? "new cont" : "new content";
   EXPECT_EQ_STR(want, buf->ptr);
 
   return 0;
@@ -68,10 +68,10 @@ DEF_TEST(dynamic_heap) {
   return status;
 }
 
-DEF_TEST(static_heap) {
-  char mem[8];
+DEF_TEST(fixed_heap) {
+  char mem[STATIC_BUFFER_SIZE];
   strbuf_t *buf;
-  CHECK_NOT_NULL(buf = strbuf_create_static(mem, sizeof(mem)));
+  CHECK_NOT_NULL(buf = strbuf_create_fixed(mem, sizeof(mem)));
 
   int status = test_buffer(buf, true);
 
@@ -80,21 +80,36 @@ DEF_TEST(static_heap) {
 }
 
 DEF_TEST(dynamic_stack) {
-  strbuf_t *buf;
-  CHECK_NOT_NULL(buf = STRBUF_CREATE);
+  strbuf_t buf = {0};
+  buf = STRBUF_CREATE;
 
-  int status = test_buffer(buf, false);
+  int status = test_buffer(&buf, false);
+
+  STRBUF_DESTROY(buf);
+  return status;
+}
+
+DEF_TEST(fixed_stack) {
+  /* This somewhat unusual syntax ensures that `sizeof(b)` will return a wrong
+   * number (size of the pointer, not the buffer; usually 4 or 8, depending on
+   * architecture), failing the test. */
+  char *b = (char[STATIC_BUFFER_SIZE]){0};
+  size_t sz = STATIC_BUFFER_SIZE;
+  strbuf_t buf = {0};
+  buf = STRBUF_CREATE_FIXED(b, sz);
+
+  int status = test_buffer(&buf, true);
 
   STRBUF_DESTROY(buf);
   return status;
 }
 
 DEF_TEST(static_stack) {
-  char mem[8];
-  strbuf_t *buf;
-  CHECK_NOT_NULL(buf = STRBUF_CREATE_STATIC(mem));
+  char b[STATIC_BUFFER_SIZE];
+  strbuf_t buf = {0};
+  buf = STRBUF_CREATE_STATIC(b);
 
-  int status = test_buffer(buf, true);
+  int status = test_buffer(&buf, true);
 
   STRBUF_DESTROY(buf);
   return status;
@@ -103,8 +118,9 @@ DEF_TEST(static_stack) {
 int main(int argc, char **argv) /* {{{ */
 {
   RUN_TEST(dynamic_heap);
-  RUN_TEST(static_heap);
+  RUN_TEST(fixed_heap);
   RUN_TEST(dynamic_stack);
+  RUN_TEST(fixed_stack);
   RUN_TEST(static_stack);
 
   END_TEST;