Allow const strings to be pushed onto the error stack

diff --git a/src/lib/util/strerror.c b/src/lib/util/strerror.c
index 46b41585dce4846f3c07234107d2979e451d8e7d..2f050b8fb82575eddcc0cbcde710b73c56053333 100644 (file)
--- a/src/lib/util/strerror.c
+++ b/src/lib/util/strerror.c
@@ -81,8 +81,12 @@ static void _fr_logging_free(void *arg)
  *
  * @param[in] buffer   to clear cursor of.
  */
-static inline void fr_strerror_clear(fr_log_buffer_t *buffer)
+static inline CC_HINT(always_inline) void fr_strerror_clear(fr_log_buffer_t *buffer, bool clear_pools)
 {
+       if (clear_pools) {
+               talloc_free_children(buffer->pool_a);
+               talloc_free_children(buffer->pool_b);
+       }
        fr_dlist_talloc_init(&buffer->entries, fr_log_entry_t, list);
 }
 
@@ -114,17 +118,43 @@ static inline fr_log_buffer_t *fr_strerror_init(void)
 
                fr_thread_local_set_destructor(fr_strerror_buffer, _fr_logging_free, buffer);
 
-               fr_strerror_clear(buffer);
+               fr_strerror_clear(buffer, false);
        }
 
        return buffer;
 }
 
-/**
+/*
+ *     If last pool was pool_a, allocate from pool_b
+ *     ...and vice versa.  This prevents the pools
+ *     from leaking due to non-contiguous allocations
+ *     when we're using fr_strerror as an argument
+ *     for another message.
+ */
+static inline CC_HINT(always_inline) TALLOC_CTX *pool_alternate(fr_log_buffer_t        *buffer)
+{
+       if (buffer->pool == buffer->pool_a) {
+               buffer->pool = buffer->pool_b;
+               return buffer->pool;
+       }
+
+       return buffer->pool = buffer->pool_a;
+}
+
+static inline CC_HINT(always_inline) void pool_free_alt(fr_log_buffer_t *buffer)
+{
+       if (buffer->pool == buffer->pool_a) {
+               talloc_free_children(buffer->pool_b);
+               return;
+       }
+       talloc_free_children(buffer->pool_a);
+}
+
+/** Create an entry in the thread local logging stack, clearing all other entries
  *
  * @hidecallergraph
  */
-static fr_log_entry_t *fr_strerror_vprintf(char const *fmt, va_list ap)
+static fr_log_entry_t *strerror_vprintf(char const *fmt, va_list ap)
 {
        va_list         ap_p;
        fr_log_entry_t  *entry;
@@ -137,97 +167,29 @@ static fr_log_entry_t *fr_strerror_vprintf(char const *fmt, va_list ap)
         *      Clear any existing log messages
         */
        if (!fmt) {
-               talloc_free_children(buffer->pool);
-               fr_strerror_clear(buffer);
-
+               fr_strerror_clear(buffer, true);
                return NULL;
        }
 
-       /*
-        *      If last pool was pool_a, allocate from pool_b
-        */
-       if (buffer->pool == buffer->pool_a) {
-               entry = talloc_zero(buffer->pool_b, fr_log_entry_t);
-               if (!entry) {
-               oom:
-                       fr_perror("Failed allocating memory for libradius error buffer");
-                       return NULL;
-               }
-
-               va_copy(ap_p, ap);
-               entry->msg = fr_vasprintf(entry, fmt, ap_p);
-               va_end(ap_p);
-               if (!entry->msg) goto oom;
-
-               talloc_free_children(buffer->pool);
-               buffer->pool = buffer->pool_b;
-       /*
-        *      ...and vice versa.  This prevents the pools
-        *      from leaking due to non-contiguous allocations
-        *      when we're using fr_strerror as an argument
-        *      for another message.
-        */
-       } else {
-               entry = talloc_zero(buffer->pool_a, fr_log_entry_t);
-               if (!entry) goto oom;
-
-               va_copy(ap_p, ap);
-               entry->msg = fr_vasprintf(entry, fmt, ap_p);
-               va_end(ap_p);
-               if (!entry->msg) goto oom;
-
-               talloc_free_children(buffer->pool);
-               buffer->pool = buffer->pool_a;
+       entry = talloc_zero(pool_alternate(buffer), fr_log_entry_t);
+       if (!entry) {
+       oom:
+               fr_perror("Failed allocating memory for libradius error buffer");
+               return NULL;
        }
 
-       fr_strerror_clear(buffer);
+       va_copy(ap_p, ap);
+       entry->msg = fr_vasprintf(entry, fmt, ap_p);
+       va_end(ap_p);
+       if (!entry->msg) goto oom;
+
+       pool_free_alt(buffer);
+       fr_strerror_clear(buffer, false);
        fr_dlist_insert_tail(&buffer->entries, entry);
 
        return entry;
 }
 
-/** Log to thread local error buffer
- *
- * @param[in] fmt      printf style format string.
- *                     If NULL clears any existing messages.
- * @param[in] ...      Arguments for the format string.
- *
- * @hidecallergraph
- */
-void fr_strerror_printf(char const *fmt, ...)
-{
-       va_list         ap;
-
-       va_start(ap, fmt);
-       fr_strerror_vprintf(fmt, ap);
-       va_end(ap);
-}
-
-/** Add an error marker to an existing stack of messages
- *
- * @param[in] subject  to mark up.
- * @param[in] offset   Positive offset to show where the error
- *                     should be positioned.
- * @param[in] fmt      Error string.
- * @param[in] ...      Arguments for the error string.
- *
- * @hidecallergraph
- */
-void fr_strerror_marker_printf(char const *subject, size_t offset, char const *fmt, ...)
-{
-       va_list         ap;
-       fr_log_entry_t  *entry;
-
-       va_start(ap, fmt);
-       entry = fr_strerror_vprintf(fmt, ap);
-       va_end(ap);
-
-       if (!entry) return;
-
-       entry->subject = talloc_strdup(entry, subject);
-       entry->offset = offset;
-}
-
 /** Add a message to an existing stack of messages
  *
  * @param[in] fmt      printf style format string.
@@ -235,17 +197,13 @@ void fr_strerror_marker_printf(char const *subject, size_t offset, char const *f
  *
  * @hidecallergraph
  */
-static fr_log_entry_t *strerror_vprintf_push(char const *fmt, va_list ap)
+static fr_log_entry_t *strerror_vprintf_push(fr_log_buffer_t *buffer, char const *fmt, va_list ap)
 {
        va_list         ap_p;
        fr_log_entry_t  *entry;
-       fr_log_buffer_t *buffer;
 
        if (!fmt) return NULL;
 
-       buffer = fr_strerror_init();
-       if (!buffer) return NULL;
-
        /*
         *      Address pathological case where we could leak memory
         *      if only a combination of fr_strerror and
@@ -253,7 +211,7 @@ static fr_log_entry_t *strerror_vprintf_push(char const *fmt, va_list ap)
         */
        if (!fr_dlist_num_elements(&buffer->entries)) talloc_free_children(buffer->pool);
 
-       entry = talloc_zero(buffer->pool_b, fr_log_entry_t);
+       entry = talloc_zero(pool_alternate(buffer), fr_log_entry_t);
        if (!entry) {
        oom:
                fr_perror("Failed allocating memory for libradius error buffer");
@@ -268,7 +226,24 @@ static fr_log_entry_t *strerror_vprintf_push(char const *fmt, va_list ap)
        return entry;
 }
 
-/** Add a message to an existing stack of messages
+/** Log to thread local error buffer
+ *
+ * @param[in] fmt      printf style format string.
+ *                     If NULL clears any existing messages.
+ * @param[in] ...      Arguments for the format string.
+ *
+ * @hidecallergraph
+ */
+void fr_strerror_printf(char const *fmt, ...)
+{
+       va_list         ap;
+
+       va_start(ap, fmt);
+       strerror_vprintf(fmt, ap);
+       va_end(ap);
+}
+
+/** Add a message to an existing stack of messages at the tail
  *
  * @param[in] fmt      printf style format string.
  *                     If NULL clears any existing messages.
@@ -286,7 +261,7 @@ void fr_strerror_printf_push(char const *fmt, ...)
        if (!buffer) return;
 
        va_start(ap, fmt);
-       entry = strerror_vprintf_push(fmt, ap);
+       entry = strerror_vprintf_push(buffer, fmt, ap);
        va_end(ap);
 
        if (!entry) return;
@@ -294,7 +269,7 @@ void fr_strerror_printf_push(char const *fmt, ...)
        fr_dlist_insert_tail(&buffer->entries, entry);
 }
 
-/** Add a message to an existing stack of messages
+/** Add a message to an existing stack of messages at the head
  *
  * @param[in] fmt      printf style format string.
  *                     If NULL clears any existing messages.
@@ -312,7 +287,7 @@ void fr_strerror_printf_push_head(char const *fmt, ...)
        if (!buffer) return;
 
        va_start(ap, fmt);
-       entry = strerror_vprintf_push(fmt, ap);
+       entry = strerror_vprintf_push(buffer, fmt, ap);
        va_end(ap);
 
        if (!entry) return;
@@ -330,6 +305,31 @@ void fr_strerror_printf_push_head(char const *fmt, ...)
  *
  * @hidecallergraph
  */
+void fr_strerror_marker_printf(char const *subject, size_t offset, char const *fmt, ...)
+{
+       va_list         ap;
+       fr_log_entry_t  *entry;
+
+       va_start(ap, fmt);
+       entry = strerror_vprintf(fmt, ap);
+       va_end(ap);
+
+       if (!entry) return;
+
+       entry->subject = talloc_strdup(entry, subject);
+       entry->offset = offset;
+}
+
+/** Add an error marker to an existing stack of messages at the tail
+ *
+ * @param[in] subject  to mark up.
+ * @param[in] offset   Positive offset to show where the error
+ *                     should be positioned.
+ * @param[in] fmt      Error string.
+ * @param[in] ...      Arguments for the error string.
+ *
+ * @hidecallergraph
+ */
 void fr_strerror_marker_printf_push(char const *subject, size_t offset, char const *fmt, ...)
 {
        va_list                 ap;
@@ -340,7 +340,7 @@ void fr_strerror_marker_printf_push(char const *subject, size_t offset, char con
        if (!buffer) return;
 
        va_start(ap, fmt);
-       entry = strerror_vprintf_push(fmt, ap);
+       entry = strerror_vprintf_push(buffer, fmt, ap);
        va_end(ap);
 
        if (!entry) return;
@@ -351,8 +351,7 @@ void fr_strerror_marker_printf_push(char const *subject, size_t offset, char con
        fr_dlist_insert_tail(&buffer->entries, entry);
 }
 
-
-/** Add an error marker to an existing stack of messages
+/** Add an error marker to an existing stack of messages at the head
  *
  * @param[in] subject  to mark up.
  * @param[in] offset   Positive offset to show where the error
@@ -372,7 +371,7 @@ void fr_strerror_marker_printf_push_head(char const *subject, size_t offset, cha
        if (!buffer) return;
 
        va_start(ap, fmt);
-       entry = strerror_vprintf_push(fmt, ap);
+       entry = strerror_vprintf_push(buffer, fmt, ap);
        va_end(ap);
 
        if (!entry) return;
@@ -383,6 +382,125 @@ void fr_strerror_marker_printf_push_head(char const *subject, size_t offset, cha
        fr_dlist_insert_head(&buffer->entries, entry);
 }
 
+/** Create an entry in the thread local logging stack using a const string, clearing all other entries
+ *
+ * @hidecallergraph
+ */
+static fr_log_entry_t *strerror_const(char const *msg)
+{
+       fr_log_entry_t  *entry;
+       fr_log_buffer_t *buffer;
+
+       buffer = fr_strerror_init();
+       if (!buffer) return NULL;
+
+       /*
+        *      Clear any existing log messages
+        */
+       if (!msg) {
+               fr_strerror_clear(buffer, true);
+               return NULL;
+       }
+
+       entry = talloc_zero(pool_alternate(buffer), fr_log_entry_t);
+       if (!entry) {
+               fr_perror("Failed allocating memory for libradius error buffer");
+               return NULL;
+       }
+       memcpy(&entry->msg, &msg, sizeof(entry->msg));
+
+       pool_free_alt(buffer);
+       fr_strerror_clear(buffer, false);
+       fr_dlist_insert_tail(&buffer->entries, entry);
+
+       return entry;
+}
+
+/** Log to thread local error buffer
+ *
+ * @param[in] msg      To add to error stack. Must have a
+ *                     lifetime equal to that of the program.
+ * @hidecallergraph
+ */
+void fr_strerror_const(char const *msg)
+{
+       (void)strerror_const(msg);
+}
+
+/** Add a message to an existing stack of messages
+ *
+ * @param[in] msg      To add to error stack. Must have a
+ *                     lifetime equal to that of the program.
+ *
+ * @hidecallergraph
+ */
+static fr_log_entry_t *strerror_const_push(fr_log_buffer_t *buffer, char const *msg)
+{
+       fr_log_entry_t  *entry;
+
+       if (!msg) return NULL;
+
+       /*
+        *      Address pathological case where we could leak memory
+        *      if only a combination of fr_strerror and
+        *      fr_strerror_printf_push are used.
+        */
+       if (!fr_dlist_num_elements(&buffer->entries)) talloc_free_children(buffer->pool);
+
+       entry = talloc_zero(pool_alternate(buffer), fr_log_entry_t);
+       if (!entry) {
+               fr_perror("Failed allocating memory for libradius error buffer");
+               return NULL;
+       }
+       memcpy(&entry->msg, &msg, sizeof(entry->msg));
+
+       return entry;
+}
+
+/** Add a message to an existing stack of messages at the tail
+ *
+ * @param[in] msg      To add to error stack. Must have a
+ *                     lifetime equal to that of the program.
+ *
+ * @hidecallergraph
+ */
+void fr_strerror_const_push(char const *msg)
+{
+       fr_log_buffer_t         *buffer;
+       fr_log_entry_t          *entry;
+
+       buffer = fr_strerror_init();
+       if (!buffer) return;
+
+       entry = strerror_const_push(buffer, msg);
+
+       if (!entry) return;
+
+       fr_dlist_insert_tail(&buffer->entries, entry);
+}
+
+/** Add a message to an existing stack of messages at the head
+ *
+ * @param[in] msg      To add to error stack. Must have a
+ *                     lifetime equal to that of the program.
+ *
+ * @hidecallergraph
+ */
+void fr_strerror_const_push_head(char const *msg)
+{
+       fr_log_buffer_t         *buffer;
+       fr_log_entry_t          *entry;
+
+       buffer = fr_strerror_init();
+       if (!buffer) return;
+
+       entry = strerror_const_push(buffer, msg);
+
+       if (!entry) return;
+
+       fr_dlist_insert_head(&buffer->entries, entry);
+}
+
 /** Get the last library error
  *
  * Will only return the last library error once, after which it will return a zero length string.
@@ -407,7 +525,7 @@ char const *fr_strerror(void)
         *      Memory gets freed on next call to
         *      fr_strerror_printf or fr_strerror_printf_push.
         */
-       fr_strerror_clear(buffer);
+       fr_strerror_clear(buffer, false);
 
        return entry->msg;
 }
@@ -437,7 +555,7 @@ char const *fr_strerror_marker(char const **subject, size_t *offset)
         *      Memory gets freed on next call to
         *      fr_strerror_printf or fr_strerror_printf_push.
         */
-       fr_strerror_clear(buffer);
+       fr_strerror_clear(buffer, false);
 
        *subject = entry->subject;
        *offset = entry->offset;

diff --git a/src/lib/util/strerror.h b/src/lib/util/strerror.h
index ee2e480c937a7f8263cb0327d2ff060c44a8cff8..47a82730697ac922ea7c9e82d624021455cb3bc8 100644 (file)
--- a/src/lib/util/strerror.h
+++ b/src/lib/util/strerror.h
@@ -36,21 +36,30 @@ extern "C" {
 /** @hidecallergraph */
 void           fr_strerror_printf(char const *fmt, ...) CC_HINT(format (printf, 1, 2));
 
-/** @hidecallergraph */
-void           fr_strerror_marker_printf(char const *subject, size_t offset, char const *fmt, ...) CC_HINT(format (printf, 3, 4));
-
 /** @hidecallergraph */
 void           fr_strerror_printf_push(char const *fmt, ...)  CC_HINT(format (printf, 1, 2));
 
 /** @hidecallergraph */
 void           fr_strerror_printf_push_head(char const *fmt, ...)  CC_HINT(format (printf, 1, 2));
 
+/** @hidecallergraph */
+void           fr_strerror_marker_printf(char const *subject, size_t offset, char const *fmt, ...) CC_HINT(format (printf, 3, 4));
+
 /** @hidecallergraph */
 void           fr_strerror_marker_printf_push(char const *subject, size_t offset, char const *fmt, ...) CC_HINT(format (printf, 3, 4));
 
 /** @hidecallergraph */
 void           fr_strerror_marker_printf_push_head(char const *subject, size_t offset, char const *fmt, ...) CC_HINT(format (printf, 3, 4));
 
+/** @hidecallergraph */
+void           fr_strerror_const(char const *msg);
+
+/** @hidecallergraph */
+void           fr_strerror_const_push(char const *msg);
+
+/** @hidecallergraph */
+void           fr_strerror_const_push_head(char const *msg);
+
 /** @hidecallergraph */
 char const     *fr_strerror(void);
 

diff --git a/src/lib/util/strerror_tests.c b/src/lib/util/strerror_tests.c
index f44526d515d21abd24380a87c769c029c399afb5..49dd74f94bde8332536f0a9e07dbb33962da7b75 100644 (file)
--- a/src/lib/util/strerror_tests.c
+++ b/src/lib/util/strerror_tests.c
@@ -25,7 +25,7 @@
 
 #include "strerror.c"
 
-static void test_strerror_uninit(void)
+static void strerror_uninit(void)
 {
        char const *error;
 
@@ -35,7 +35,7 @@ static void test_strerror_uninit(void)
        TEST_CHECK(error && (error[0] == '\0'));
 }
 
-static void test_strerror_pop_uninit(void)
+static void strerror_pop_uninit(void)
 {
        char const *error;
 
@@ -44,7 +44,7 @@ static void test_strerror_pop_uninit(void)
        TEST_CHECK(error == NULL);
 }
 
-static void test_strerror_printf(void)
+static void strerror_printf(void)
 {
        char const *error;
 
@@ -60,7 +60,7 @@ static void test_strerror_printf(void)
        TEST_CHECK(error && (error[0] == '\0'));
 }
 
-static void test_strerror_printf_push_pop(void)
+static void strerror_printf_push_pop(void)
 {
        char const *error;
 
@@ -78,7 +78,7 @@ static void test_strerror_printf_push_pop(void)
        TEST_CHECK(error && (error[0] == '\0'));
 }
 
-static void test_strerror_printf_push_strerror(void)
+static void strerror_printf_push_strerror(void)
 {
        char const *error;
 
@@ -92,7 +92,7 @@ static void test_strerror_printf_push_strerror(void)
        TEST_CHECK(error == NULL);
 }
 
-static void test_strerror_printf_push_pop_multi(void)
+static void strerror_printf_push_pop_multi(void)
 {
        char const *error;
 
@@ -115,7 +115,7 @@ static void test_strerror_printf_push_pop_multi(void)
        TEST_CHECK(error && (error[0] == '\0'));
 }
 
-static void test_strerror_printf_push_strerror_multi(void)
+static void strerror_printf_push_strerror_multi(void)
 {
        char const *error;
 
@@ -130,7 +130,7 @@ static void test_strerror_printf_push_strerror_multi(void)
        TEST_CHECK(error == NULL);
 }
 
-static void test_strerror_printf_strerror_append(void)
+static void strerror_printf_strerror_append(void)
 {
        char const *error;
 
@@ -146,7 +146,7 @@ static void test_strerror_printf_strerror_append(void)
        TEST_CHECK(error && (error[0] == '\0'));
 }
 
-static void test_strerror_printf_push_append(void)
+static void strerror_printf_push_append(void)
 {
        char const *error;
 
@@ -162,7 +162,7 @@ static void test_strerror_printf_push_append(void)
        TEST_CHECK(error && (error[0] == '\0'));
 }
 
-static void test_strerror_printf_push_append2(void)
+static void strerror_printf_push_append2(void)
 {
        char const *error;
 
@@ -179,18 +179,18 @@ static void test_strerror_printf_push_append2(void)
 }
 
 TEST_LIST = {
-       { "test_strerror_uninit",                       test_strerror_uninit },
-       { "test_strerror_pop_uninit",                   test_strerror_pop_uninit },
-
-       { "test_strerror_printf",                       test_strerror_printf },
-       { "test_strerror_printf_push_pop",              test_strerror_printf_push_pop },
-
-       { "test_strerror_printf_push_strerror",         test_strerror_printf_push_strerror },
-       { "test_strerror_printf_push_pop_multi",        test_strerror_printf_push_pop_multi },
-       { "test_strerror_printf_push_strerror_multi",   test_strerror_printf_push_strerror_multi },
-       { "test_strerror_printf_strerror_append",       test_strerror_printf_strerror_append },
-       { "test_strerror_printf_push_append",           test_strerror_printf_push_append },
-       { "test_strerror_printf_push_append2",          test_strerror_printf_push_append2 },
+       { "strerror_uninit",                    strerror_uninit },
+       { "strerror_pop_uninit",                strerror_pop_uninit },
+
+       { "strerror_printf",                    strerror_printf },
+       { "strerror_printf_push_pop",           strerror_printf_push_pop },
+
+       { "strerror_printf_push_strerror",      strerror_printf_push_strerror },
+       { "strerror_printf_push_pop_multi",     strerror_printf_push_pop_multi },
+       { "strerror_printf_push_strerror_multi",strerror_printf_push_strerror_multi },
+       { "strerror_printf_strerror_append",    strerror_printf_strerror_append },
+       { "strerror_printf_push_append",        strerror_printf_push_append },
+       { "strerror_printf_push_append2",       strerror_printf_push_append2 },
 
        { 0 }
 };