Fixed realloc OOM bug introduced due to confusing code, added comment…

… for future. umm_info(NULL, true) is now treated as a debug option that is enabled by Arduino IDE->tools `Debug port: "Serial"`, etc. It now prints when DEBUG_ESP_PORT or UMM_INFO_PRINT are defined. Cleanup comments and some conditionals.
esp8266 · devyte · Oct 29, 2019 · Aug 20, 2019 · Aug 20, 2019 · Aug 23, 2019
commit 9fcf27b084b59002ee9b1036307cd2f987c2071c
diff --git a/cores/esp8266/heap.cpp b/cores/esp8266/heap.cpp
@@ -142,8 +142,8 @@ void ICACHE_RAM_ATTR print_loc(size_t size, const char* file, int line)
 #else  // ! DEBUG_ESP_OOM
 
 #if 1
-//C Skip OOM check of pvPort... . It cost 64 more bytes of IRAM to turn on.
-//C Was not previously enabled.
+//C Skip OOM logging of last fail for malloc/... and pvPort... .
+//C It cost 64 more bytes of IRAM to turn on. And was not previously enabled.
 #undef PTR_CHECK__LOG_LAST_FAIL
 #define PTR_CHECK__LOG_LAST_FAIL(p, s, a)
 #endif

diff --git a/cores/esp8266/umm_malloc/isr_safe_printf.cpp b/cores/esp8266/umm_malloc/isr_safe_printf.cpp
@@ -64,7 +64,7 @@
 #include "umm_malloc_cfg.h"
 extern "C" {
 
-#if defined(DEBUG_ESP_PORT) || defined(DEBUG_ESP_OOM) || defined(UMM_POISON_CHECK) || defined(UMM_POISON_CHECK_LITE)
+#if defined(DEBUG_ESP_ISR)
 
 int _isr_safe_printf_P(const char *fmt, ...) __attribute__((format(printf, 1, 2)));
 // Note, _isr_safe_printf_P will not handle additional string arguments in

diff --git a/cores/esp8266/umm_malloc/umm_local.h b/cores/esp8266/umm_malloc/umm_local.h
@@ -13,6 +13,14 @@
 #define memmove ets_memmove
 #define memset ets_memset
 
+#if !defined(UMM_INFO_PRINT)
+/* This will stop umm_info from printing.
+ * At this time DBGLOG_FORCE is only used by umm_info.
+ */
+#undef DBGLOG_FORCE
+#define DBGLOG_FORCE(force, format, ...) {(void)force; (void)format;}
+#endif
+
 #if defined(DEBUG_ESP_OOM) || defined(UMM_POISON_CHECK) || defined(UMM_POISON_CHECK_LITE) || defined(UMM_INTEGRITY_CHECK)
 #else
 

diff --git a/cores/esp8266/umm_malloc/umm_malloc.cpp b/cores/esp8266/umm_malloc/umm_malloc.cpp
@@ -771,12 +771,15 @@ void *umm_realloc( void *ptr, size_t size ) {
             memcpy( ptr, oldptr, curSize );
             UMM_CRITICAL_RESUME(id_realloc);
             umm_free_core( oldptr );
-            blockSize = blocks;
         } else {
             DBGLOG_DEBUG( "realloc %i to a bigger block %i failed - return NULL and leave the old block!\n", blockSize, blocks );
             /* This space intentionally left blnk */
             STATS__OOM_UPDATE();
         }
+        /* This is not accurate for OOM case; however, it will work for
+         * stopping a call to free before return.
+         */
+        blockSize = blocks;
     }
 #elif defined(UMM_REALLOC_DEFRAG)
   /*
@@ -847,12 +850,15 @@ void *umm_realloc( void *ptr, size_t size ) {
             memcpy( ptr, oldptr, curSize );
             UMM_CRITICAL_RESUME(id_realloc);
             umm_free_core( oldptr);
-            blockSize = blocks;
         } else {
             DBGLOG_DEBUG( "realloc %d to a bigger block %d failed - return NULL and leave the old block!\n", blockSize, blocks );
             /* This space intentionally left blnk */
             STATS__OOM_UPDATE();
         }
+        /* This is not accurate for OOM case; however, it will work for
+         * stopping a call to free before return.
+         */
+        blockSize = blocks;
     }
 #else
 #warning "Neither UMM_REALLOC_DEFRAG nor UMM_REALLOC_MINIMIZE_COPY is defined - check umm_malloc_cfg.h"
@@ -869,13 +875,15 @@ void *umm_realloc( void *ptr, size_t size ) {
             memcpy( ptr, oldptr, curSize );
             UMM_CRITICAL_RESUME(id_realloc);
             umm_free_core( oldptr );
-            blockSize = blocks;
         } else {
             DBGLOG_DEBUG( "realloc %d to a bigger block %d failed - return NULL and leave the old block!\n", blockSize, blocks );
             /* This space intentionally left blnk */
-            UMM_CRITICAL_RESUME(id_realloc);
             STATS__OOM_UPDATE();
         }
+        /* This is not accurate for OOM case; however, it will work for
+         * stopping a call to free before return.
+         */
+        blockSize = blocks;
     }
 #endif
     /* Now all we need to do is figure out if the block fit exactly or if we

diff --git a/cores/esp8266/umm_malloc/umm_malloc_cfg.h b/cores/esp8266/umm_malloc/umm_malloc_cfg.h
@@ -114,6 +114,23 @@ extern char _heap_start[];
 #else
 #endif
 
+/*
+ * -D UMM_INFO_PRINT
+ *
+ * umm_info(NULL, true) is used to print a debug view of heap information to the
+ * debug port. It has to walk the heap and print out information while in a
+ * critical section. This requires that the print function be able to print w/o
+ * doing malloc calls and from an IRQ disabled context. This requires extra IRAM
+ * based code for printing. Without this define `umm_info(NULL, true)` will
+ * not print.
+ *
+ * UMM_INFO_PRINT is enabled as part of selecting `Debug port: "Serial" or
+ * "Serial1"`. To make available all the time use '-D UMM_INFO_PRINT`.
+ */
+#if defined(DEBUG_ESP_PORT) && !defined(UMM_INFO_PRINT)
+#define UMM_INFO_PRINT
+#endif
+
 /*
  * -D UMM_STATS :
  *
@@ -541,17 +558,23 @@ static inline void _critical_exit(UMM_TIME_STAT *p, uint32_t *saved_ps) {
 #undef DBGLOG_FUNCTION
 #undef DBGLOG_FUNCTION_P
 
-#if defined(DEBUG_ESP_PORT) || defined(DEBUG_ESP_OOM) || defined(UMM_POISON_CHECK) || defined(UMM_POISON_CHECK_LITE)
+#if defined(DEBUG_ESP_PORT) || defined(DEBUG_ESP_OOM) || \
+    defined(UMM_POISON_CHECK) || defined(UMM_POISON_CHECK_LITE) || \
+    defined(UMM_INFO_PRINT) || defined(UMM_INTEGRITY_CHECK)
+
+#ifndef DEBUG_ESP_ISR
+#define DEBUG_ESP_ISR
+#endif
+
 int _isr_safe_printf_P(const char *fmt, ...) __attribute__((format(printf, 1, 2)));
 // Note, _isr_safe_printf_P will not handle additional string arguments in
 // PROGMEM. Only the 1st parameter, fmt, is supported in PROGMEM.
 #define DBGLOG_FUNCTION(fmt, ...) _isr_safe_printf_P(PSTR(fmt), ##__VA_ARGS__)
 #define DBGLOG_FUNCTION_P(fmt, ...) _isr_safe_printf_P(fmt, ##__VA_ARGS__)
 #else
-#define DBGLOG_FUNCTION(fmt, ...) printf(PSTR(fmt), ##__VA_ARGS__)
-#define DBGLOG_FUNCTION_P(fmt, ...) printf_P(fmt, ##__VA_ARGS__)
+#define DBGLOG_FUNCTION(fmt, ...)   do { (void)fmt; } while(false)
+#define DBGLOG_FUNCTION_P(fmt, ...) do { (void)fmt; } while(false)
 #endif
-//C What about printing from umm_info - does it need to be ISR safe
 
 /////////////////////////////////////////////////