Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove deprecation diagnostic supppression for dtostrf #212

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions api/deprecated-avr-comp/avr/dtostrf.c.impl
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

27 changes: 23 additions & 4 deletions test/src/dtostrf.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@
* INCLUDE
**************************************************************************************/

#include <api/deprecated-avr-comp/avr/dtostrf.h>

#include <stdlib.h>
#include <cstdio>
#include <cfloat>

/**************************************************************************************
* FUNCTION IMPLEMENTATION
Expand All @@ -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"
Expand Down