buflib: Refactor various debugging features

Gate buflib_get_data() checking, debug printing, and buflib
integrity checks behind individual defines in buflib.h, and
turn them all off by default. If needed, they can be turned
on manually when compiling.

The buflib debug menu is only available if debug printing is
enabled, so after this commit it will no longer be included
in normal builds -- it isn't very useful to end users.

Change-Id: Iab25b7852bc7c5592ce04c9c45762046a87d5bc3

7 files changed, 75 insertions, 43 deletions

diff --git a/apps/debug_menu.c b/apps/debug_menu.c
index 65b660813f..070c08f4f6 100644
--- a/apps/debug_menu.c
+++ b/apps/debug_menu.c
@@ -496,6 +496,7 @@ static bool dbg_buffering_thread(void)
 #undef STR_DATAREM
 }
 
+#ifdef BUFLIB_DEBUG_PRINT
 static const char* bf_getname(int selected_item, void *data,
                                    char *buffer, size_t buffer_len)
 {
@@ -536,6 +537,7 @@ static bool dbg_buflib_allocs(void)
     info.timeout = HZ;
     return simplelist_show_list(&info);
 }
+#endif /* BUFLIB_DEBUG_PRINT */
 
 #if (CONFIG_PLATFORM & PLATFORM_NATIVE)
 static const char* dbg_partitions_getname(int selected_item, void *data,
@@ -2619,7 +2621,9 @@ static const struct {
 #ifdef PM_DEBUG
         { "pm histogram", peak_meter_histogram},
 #endif /* PM_DEBUG */
+#ifdef BUFLIB_DEBUG_PRINT
         { "View buflib allocs", dbg_buflib_allocs },
+#endif
 #ifndef SIMULATOR
 #if CONFIG_TUNER
         { "FM Radio", dbg_fm_radio },
diff --git a/firmware/buflib.c b/firmware/buflib.c
index be1ff01af7..0f8836b3b7 100644
--- a/firmware/buflib.c
+++ b/firmware/buflib.c
@@ -991,8 +991,7 @@ unsigned buflib_pin_count(struct buflib_context *ctx, int handle)
     return data[idx_PIN].pincount;
 }
 
-#ifdef DEBUG
-
+#ifdef BUFLIB_DEBUG_GET_DATA
 void *buflib_get_data(struct buflib_context *ctx, int handle)
 {
     if (handle <= 0)
@@ -1000,7 +999,9 @@ void *buflib_get_data(struct buflib_context *ctx, int handle)
 
     return (void*)(ctx->handle_table[-handle].alloc);
 }
+#endif
 
+#ifdef BUFLIB_DEBUG_CHECK_VALID
 void buflib_check_valid(struct buflib_context *ctx)
 {
     for(union buflib_data *block = ctx->buf_start;
@@ -1016,10 +1017,11 @@ void buflib_check_valid(struct buflib_context *ctx)
 }
 #endif
 
-#ifdef BUFLIB_DEBUG_BLOCK_SINGLE
+#ifdef BUFLIB_DEBUG_PRINT
 int buflib_get_num_blocks(struct buflib_context *ctx)
 {
     int i = 0;
+
     for(union buflib_data *block = ctx->buf_start;
         block < ctx->alloc_end;
         block += abs(block->val))
@@ -1027,11 +1029,12 @@ int buflib_get_num_blocks(struct buflib_context *ctx)
         check_block_length(ctx, block);
         ++i;
     }
+
     return i;
 }
 
-void buflib_print_block_at(struct buflib_context *ctx, int block_num,
-                           char* buf, size_t bufsize)
+bool buflib_print_block_at(struct buflib_context *ctx, int block_num,
+                           char *buf, size_t bufsize)
 {
     for(union buflib_data *block = ctx->buf_start;
         block < ctx->alloc_end;
@@ -1044,8 +1047,13 @@ void buflib_print_block_at(struct buflib_context *ctx, int block_num,
             snprintf(buf, bufsize, "%8p: val: %4ld (%sallocated)",
                      block, (long)block->val,
                      block->val > 0 ? "" : "un");
+            return true;
         }
     }
+
+    if (bufsize > 0)
+        *buf = '\0';
+    return false;
 }
 #endif
 
diff --git a/firmware/core_alloc.c b/firmware/core_alloc.c
index f5cc7f3189..948911b973 100644
--- a/firmware/core_alloc.c
+++ b/firmware/core_alloc.c
@@ -35,8 +35,11 @@ unsigned char *audiobufend = audiobuffer + sizeof(audiobuffer);
 extern unsigned char *audiobufend;
 #endif
 
+#ifdef BUFLIB_DEBUG_PRINT
 /* debug test alloc */
 static int test_alloc;
+#endif
+
 void core_allocator_init(void)
 {
     unsigned char *start = ALIGN_UP(audiobuffer, sizeof(intptr_t));
@@ -51,16 +54,9 @@ void core_allocator_init(void)
 
     buflib_init(&core_ctx, start, audiobufend - start);
 
+#ifdef BUFLIB_DEBUG_PRINT
     test_alloc = core_alloc(112);
-}
-
-bool core_test_free(void)
-{
-    bool ret = test_alloc > 0;
-    if (ret)
-        test_alloc = core_free(test_alloc);
-
-    return ret;
+#endif
 }
 
 /* Allocate memory in the "core" context. See documentation
@@ -119,17 +115,28 @@ unsigned core_pin_count(int handle)
     return buflib_pin_count(&core_ctx, handle);
 }
 
+#ifdef BUFLIB_DEBUG_PRINT
 int core_get_num_blocks(void)
 {
     return buflib_get_num_blocks(&core_ctx);
 }
 
-void core_print_block_at(int block_num, char* buf, size_t bufsize)
+bool core_print_block_at(int block_num, char* buf, size_t bufsize)
 {
-    buflib_print_block_at(&core_ctx, block_num, buf, bufsize);
+    return buflib_print_block_at(&core_ctx, block_num, buf, bufsize);
 }
 
-#ifdef DEBUG
+bool core_test_free(void)
+{
+    bool ret = test_alloc > 0;
+    if (ret)
+        test_alloc = core_free(test_alloc);
+
+    return ret;
+}
+#endif
+
+#ifdef BUFLIB_DEBUG_CHECK_VALID
 void core_check_valid(void)
 {
     buflib_check_valid(&core_ctx);
diff --git a/firmware/include/buflib.h b/firmware/include/buflib.h
index 349e4a3e7a..49e4db11c0 100644
--- a/firmware/include/buflib.h
+++ b/firmware/include/buflib.h
@@ -23,15 +23,21 @@
 * KIND, either express or implied.
 *
 ****************************************************************************/
-
 #ifndef _BUFLIB_H_
 #define _BUFLIB_H_
+
 #include <stdint.h>
 #include <stdbool.h>
 #include <string.h>
 
-/* enable single block debugging */
-#define BUFLIB_DEBUG_BLOCK_SINGLE
+/* add extra checks to buflib_get_data to catch bad handles */
+//#define BUFLIB_DEBUG_GET_DATA
+
+/* support integrity check */
+//#define BUFLIB_DEBUG_CHECK_VALID
+
+/* support debug printing of memory blocks */
+//#define BUFLIB_DEBUG_PRINT
 
 union buflib_data
 {
@@ -260,12 +266,12 @@ int buflib_alloc_maximum(struct buflib_context* ctx,
  *
  * Returns: The start pointer of the allocation
  */
-#ifdef DEBUG
-void* buflib_get_data(struct buflib_context *ctx, int handle);
+#ifdef BUFLIB_DEBUG_GET_DATA
+void *buflib_get_data(struct buflib_context *ctx, int handle);
 #else
-static inline void* buflib_get_data(struct buflib_context *ctx, int handle)
+static inline void *buflib_get_data(struct buflib_context *ctx, int handle)
 {
-    return (void*)(ctx->handle_table[-handle].alloc);
+    return (void *)ctx->handle_table[-handle].alloc;
 }
 #endif
 
@@ -342,29 +348,34 @@ void* buflib_buffer_out(struct buflib_context *ctx, size_t *size);
  */
 void buflib_buffer_in(struct buflib_context *ctx, int size);
 
-/* debugging */
-
+#ifdef BUFLIB_DEBUG_PRINT
 /**
- * Gets the number of blocks in the entire buffer, allocated or unallocated
+ * Return the number of blocks in the buffer, allocated or unallocated.
  *
- * Only available if BUFLIB_DEBUG_BLOCK_SIGNLE is defined
+ * Only available if BUFLIB_DEBUG_PRINT is defined.
  */
 int buflib_get_num_blocks(struct buflib_context *ctx);
 
 /**
- * Print information about a single block as indicated by block_num
- * into buf
+ * Write a string describing the block at index block_num to the
+ * provided buffer. The buffer will always be null terminated and
+ * there is no provision to detect truncation. (A 40-byte buffer
+ * is enough to contain any returned string.)
  *
- * buflib_get_num_blocks() beforehand to get the total number of blocks,
- * as passing an block_num higher than that is undefined
+ * Returns false if the block index is out of bounds, and writes
+ * an empty string.
  *
- * Only available if BUFLIB_DEBUG_BLOCK_SIGNLE is defined
+ * Only available if BUFLIB_DEBUG_PRINT is defined.
  */
-void buflib_print_block_at(struct buflib_context *ctx, int block_num,
-                            char* buf, size_t bufsize);
+bool buflib_print_block_at(struct buflib_context *ctx, int block_num,
+                           char *buf, size_t bufsize);
+#endif
 
+#ifdef BUFLIB_DEBUG_CHECK_VALID
 /**
  * Check integrity of given buflib context
  */
 void buflib_check_valid(struct buflib_context *ctx);
 #endif
+
+#endif /* _BUFLIB_H_ */
diff --git a/firmware/include/core_alloc.h b/firmware/include/core_alloc.h
index 8011c227b1..22cc1988da 100644
--- a/firmware/include/core_alloc.h
+++ b/firmware/include/core_alloc.h
@@ -21,21 +21,23 @@ unsigned core_pin_count(int handle);
 int core_free(int handle);
 size_t core_available(void);
 size_t core_allocatable(void);
-#ifdef DEBUG
+
+#ifdef BUFLIB_DEBUG_CHECK_VALID
 void core_check_valid(void);
 #endif
 
 /* DO NOT ADD wrappers for buflib_buffer_out/in. They do not call
  * the move callbacks and are therefore unsafe in the core */
 
-#ifdef BUFLIB_DEBUG_BLOCK_SINGLE
-int  core_get_num_blocks(void);
-void core_print_block_at(int block_num, char* buf, size_t bufsize);
-#endif
+#ifdef BUFLIB_DEBUG_PRINT
+int core_get_num_blocks(void);
+bool core_print_block_at(int block_num, char* buf, size_t bufsize);
 
 /* frees the debug test alloc created at initialization,
- * since this is the first any further alloc should force a compaction run */
+ * since this is the first any further alloc should force a compaction run
+ * only used if debug print is active */
 bool core_test_free(void);
+#endif
 
 static inline void* core_get_data(int handle)
 {
diff --git a/firmware/kernel/thread.c b/firmware/kernel/thread.c
index a422624df7..25677c79f9 100644
--- a/firmware/kernel/thread.c
+++ b/firmware/kernel/thread.c
@@ -1019,7 +1019,7 @@ void switch_thread(void)
 #ifdef RB_PROFILE
         profile_thread_stopped(THREAD_ID_SLOT(thread->id));
 #endif
-#ifdef DEBUG
+#ifdef BUFLIB_DEBUG_CHECK_VALID
         /* Check core_ctx buflib integrity */
         core_check_valid();
 #endif
diff --git a/firmware/target/hosted/sdl/thread-sdl.c b/firmware/target/hosted/sdl/thread-sdl.c
index a76941f103..17a0f847b5 100644
--- a/firmware/target/hosted/sdl/thread-sdl.c
+++ b/firmware/target/hosted/sdl/thread-sdl.c
@@ -215,7 +215,7 @@ void switch_thread(void)
         } /* STATE_SLEEPING: */
     }
 
-#ifdef DEBUG
+#ifdef BUFLIB_DEBUG_CHECK_VALID
     core_check_valid();
 #endif
     __running_self_entry() = current;