From fdda28cfe40701deb71632faa403cde86695d737 Mon Sep 17 00:00:00 2001
From: Jeremy Boynes <jeremy@boynes.com>
Date: Sat, 9 Sep 2023 08:19:28 +0100
Subject: [PATCH] Remove deprecation diagnostic supppression for dtostrf

---
 api/deprecated-avr-comp/avr/dtostrf.c.impl |  3 ---
 test/src/dtostrf.cpp                       | 27 ++++++++++++++++++----
 2 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/api/deprecated-avr-comp/avr/dtostrf.c.impl b/api/deprecated-avr-comp/avr/dtostrf.c.impl
index f410886c..96987a8f 100644
--- a/api/deprecated-avr-comp/avr/dtostrf.c.impl
+++ b/api/deprecated-avr-comp/avr/dtostrf.c.impl
@@ -29,12 +29,9 @@
 char *dtostrf (double val, signed char width, unsigned char prec, char *sout) {
   asm(".global _printf_float");
 
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
   char fmt[20];
   sprintf(fmt, "%%%d.%df", width, prec);
   sprintf(sout, fmt, val);
   return sout;
-#pragma GCC diagnostic pop
 }
 
diff --git a/test/src/dtostrf.cpp b/test/src/dtostrf.cpp
index 0700debb..afa90213 100644
--- a/test/src/dtostrf.cpp
+++ b/test/src/dtostrf.cpp
@@ -6,9 +6,8 @@
  * INCLUDE
  **************************************************************************************/
 
-#include <api/deprecated-avr-comp/avr/dtostrf.h>
-
-#include <stdlib.h>
+#include <cstdio>
+#include <cfloat>
 
 /**************************************************************************************
  * FUNCTION IMPLEMENTATION
@@ -17,8 +16,28 @@
 #ifdef __cplusplus
 extern "C" {
 #endif
+/*
+ * A fundamental issue with dtostrf is the risk of buffer overflow as the size of the
+ * output buffer is not passed in. Previous implementation relied on sprintf which has
+ * the same issue and has now been deprecated, leading to compilation warnings that are
+ * considered fatal. Here, we use snprintf to avoid those warnings, with a limit
+ * set large enough for the longest buffer used by the String class. The risk
+ * of buffer overflow remains when a smaller buffer is passed in.
+ *
+ * TODO Refactor String not to rely on this function.
+ */
+char *dtostrf (double val, signed char width, unsigned char prec, char *sout) {
+
+    // From String.h - DOUBLE_BUF_SIZE is the largest it could use
+    static size_t const FLT_MAX_DECIMAL_PLACES = 10;
+    static size_t const DBL_MAX_DECIMAL_PLACES = FLT_MAX_DECIMAL_PLACES;
+    static size_t const DOUBLE_BUF_SIZE = DBL_MAX_10_EXP + DBL_MAX_DECIMAL_PLACES + 1 /* '-' */ + 1 /* '.' */ + 1 /* '\0' */;
 
-#include <api/deprecated-avr-comp/avr/dtostrf.c.impl>
+    char fmt[20];
+    snprintf(fmt, sizeof(fmt), "%%%d.%df", width, prec);
+    snprintf(sout, DOUBLE_BUF_SIZE, fmt, val);
+    return sout;
+}
 
 #ifdef __cplusplus
 } // extern "C"