s/String discussion (again)

Come on - let’s be serious a minute - The first advice is post the code …. when they have an issue…

my code is totally fine. Running for years indoor and outdoor.

So talking about real code : will you replace your strcpy that is fine with strlcpy and test for proper completion in your library? Who knows those 4 or 5 characters might have a tough time fitting in the 33 byte buffer. if you talk the talk, you should walk the walk...

or would you recognise this is uncalled for, unneeded, a good example of code that will be solid (well - you allocated the buffer on the stack - something else that can go wrong… may be need to check stack availability ?)

If you want to continue this discussion.
Post an example sketch to discuss.

here you go.

char result[33];
double d = 1.2345678901234567890;
int decs = 10;

void setup() {
  Serial.begin(115200);
  Serial.println();
  if (isnan(d))                 strcpy(result, "nan");
  else if (isinf(d))            strcpy(result, "inf");
  else if (d >= 4294967039.0)   strcpy(result, "ovf");  // constant determined empirically
  else if (d <= -4294967039.0)  strcpy(result, "-ovf"); // constant determined empirically
  else if (decs > 7)            decs = 7;               // seems to be the limit for print
  dtostrf(d, 1, decs, result);

  Serial.println(d, decs);
  Serial.println(result);
}

void loop() {}

Should I be worried about using strcpy() or dtostrf()? I've read they are really bad and will crash my sketch :wink:

Are you suggesting this is a one off 'sketch' that a user on this forum would write?
Looks more like a library function.
Actually looks like you copied that code from my SafeString library.
Is none of your own code good enough to be seen?

This is what users actually post float/double in sprintf without and with dtostrf

void setup()
{
    double dblB = 0.31830988;
    char buf[20];

    SerialUSB.begin(9600);
    delay(2000);
    SerialUSB.println("dtostrf() demo program");

    dtostrf(dblB, 0, 8, buf);
    SerialUSB.println(buf);
}

Which is why I recommend my SafeString library function, because dtostrf( ) is easy to get wrong and result in a buffer overflow.

My codes have no problem to discuss... they work...

This sample code compares how print() handles rounding versus dtostrf(). The output is

1.2345678
1.2345679

as you can see it's a different output. if that number was a (very precise) gps coordinate that I wanted to transfer in ASCII to another device, I would not get the very same exact location (close but not the same).

Also I was interested in your view on this piece of code and how you would advise to make it robust since you claim that using strcpy() or dtostrf() are risky business... (and your SafeString library is not an option since this is what it does...)

Modest as always :slight_smile:

No true, you can use the usual print( ) method as well.

Don't use strcpy, don't use dtostrf as it is prone to buffer overflows as the user code above shows.
Use the pre-prepared pre-tested SafeString library function.

So I have one for you.
How would you 'fix' this sketch to not crash on unknown input.
Use of string char causes Arduino code to restart

but I looked into the pre-tested SafeString library function and it uses exactly that code :slight_smile:

I'll use Arduino JSON library for example. Depends on your hardware capability.
I could also use regular expression parsing with Gammon Forum : Programming : General : Lua regular expression library that does not need Lua

the first question I would have asked is what was the OP intending to extract. if it's only one value you can listen into a circular buffer something that is a marker coming just before the value and then extract it

Ok where is the code? So we can discuss it complexity and suitability?

where is OP's answer to the exact need ?
Requirements usually drive the solution...

Not to crash on un-expected input length.

here you go

void setup() {}
void loop() {}

it won't crash. it meets your requirements definition...

OP said he consumes a JSON from a web API. the Arduino JSON library has capability to handle a stream directly from a WiFi Client for example and you can use filters to extract relevant data. That would probably be the easiest way to extracting data.

if there were other constraints/requirements, they were not listed.

So your solution would be to ditch all those error prone strncpy calls and use a library the user may not be familar with and which has it own memory issues.
Far simplier to just use strlcpy() instead of strncpy()
Which is what I would have suggest if that had been posted now.
strlcpy() seems a much simplier solution to avoiding the crash so he can debug the problem.

OP said

I can consume the API and have been experimenting how to parse the JSON. Accepted ArduinoJSON might be a solution but I thought, based on the amount I had to parse, a bespoke piece of code would be worth trying.

hence my comment.

I'm saying that if we had clear requirements and constraints, then we could discuss a solution.

The question raised in the post you linked to was "why is my code crashing" and the answer was because OP was not careful with buffer sizes... Your library can help identify those errors and not crash, but if OP was not testing for the error messages in code (you can't spend your life in front of the Serial monitor) and acting on those, then the behavior of the code would not be right.

So we are back at

  • ensure you don't overflow
  • test for coherence before or after messing with a buffer

in any case, testing and handling the exception is required at application level. All I've been saying is that you have choices of when/if you test, and these choice is guided by the use case.

dtostrf() can give you buffer overflows if you don't get the result char[ ] correct (or the number becomes larger than you expected. The resultLen argument to dtostrf is the MINIMUM result length NOT the maximum.

My SafeString library has an alternative that guards against buffer overflows by letting you specify the FIXED result length.
Here is an example of printing a double to a SafeString with 5 decimal places in a fixed width of 9
The print(double, ... ) also has options to justify left or right and to force a + sign for tabular alignment. Also works with int/longs just specify 0 for the decimal places.
Once you have the SafeString result you can just output it as normal
Serial.println(sfStr);

// https://forum.arduino.cc/t/printf-on-arduino/888528/6
// download SafeString V4.1.5+ library from the Arduino Library manager or from
// https://www.forward.com.au/pfod/ArduinoProgramming/SafeString/index.html
#include "SafeString.h"

void setup() {
  Serial.begin(9600);
  for (int i = 10; i > 0; i--) {
    Serial.print(' '); Serial.print(i); delay(500);
  }
  Serial.println();
  SafeString::setOutput(Serial); // enable full debugging error msgs

  Serial.println(F(" cSF(sfStr, 9); // where to print to double "));
  cSF(sfStr, 9);
  double d = -0.12345123;
  Serial.println(F(" Keep increasing the size of the value until it won't fit in width 9"));
  Serial.println();
  while (!sfStr.hasError()) { // double not too large to fit in 9
    d = d * 10;
    Serial.print(F(" Formatting value d=")); Serial.println(d, 7);
    sfStr = ""; // clear last output
    sfStr.print(d, 5, 9); // 5 is prec requested will be automatically reduced if d will not fit in width 9
    Serial.print(F(" using sfStr.print(d,5,9)     |")); // clear last output
    Serial.print(sfStr); // OR Serial.print(sfStr.c_str())
    Serial.println(F("| text following field"));
  }
  Serial.println();
  Serial.println(F("SafeString Error detected! "));
  Serial.println(F(" NOTE: when the integer number part is too large for the field, "));
  Serial.println(F("       the field is just padded with blanks are concatinated to the SafeString."));
}

void loop() {
}

The output is

cSF(sfStr, 9); // where to print to text 
 Keep increasing the size of the value until it won't fit in width 9

 Formatting value d=-1.2345123
 using sfStr.print(d,5,9)     | -1.23451| text following field
 Formatting value d=-12.3451232
 using sfStr.print(d,5,9)     |-12.34512| text following field
 Formatting value d=-123.4512329
 using sfStr.print(d,5,9)     |-123.4512| text following field
 Formatting value d=-1234.5123291
 using sfStr.print(d,5,9)     |-1234.512| text following field
 Formatting value d=-12345.1230468
 using sfStr.print(d,5,9)     |-12345.12| text following field
 Formatting value d=-123451.2343750
 using sfStr.print(d,5,9)     |-123451.2| text following field
 Formatting value d=-1234512.3750000
 using sfStr.print(d,5,9)     | -1234512| text following field
 Formatting value d=-12345124.0000000
 using sfStr.print(d,5,9)     |-12345124| text following field
 Formatting value d=-123451240.0000000
Error: sfStr.print() width:9 too small to display even just the integer part of -123451240.00
        sfStr cap:9 len:0 ''
 using sfStr.print(d,5,9)     |         | text following field

SafeString Error detected! 
 NOTE: when the integer number part is too large for the field, 
       the field is just padded with blanks are concatinated to the SafeString

so is this dtostrf() call in your library risky?

  // max chars are 11 for integer part including sign (-4294967040), + 1 (.) + 7 prec (prec limited to 7) = 19 + '\0' => 20
  // ESP32
  // max chars are 19 for integer part including sign (-9223372036854775807L), + 1 (.) + 7 prec (prec limited to 7) = 19 +1 + 7 + '\0' => 27
  char result[33]; // 19 +1 + 7 + '\0' => 27 for extra
  result[0] = '\0';
  ...
  dtostrf(d, width, decs, result);

Did you get the math right and is 33 always enough?
if you use it, can't other use it too? :innocent:

Talking about double - run this on an ESP32 or SAMD based arduino (hopefully I did not do stupid things with your library)

// RUN THIS ON AN ESP32 OR SAMD
#include <stdio.h>
#include <SafeString.h>
double d = +0.012345678901234; // 15 significant digits I care about
char dStr[20 + 1];
cSF(sfStr, 20);

void setup() {
  Serial.begin(115200);
  while (!Serial);
  SafeString::setOutput(Serial); // enable full debugging error msgs

  Serial.println();
  sfStr.print(d, 15, 20, true);
  Serial.print("SafeString result with precision 15: ["); Serial.print(sfStr); Serial.println("]");

  sfStr = ""; // as print does actually concat... confusing...
  sfStr.print(d, 7, 20, true);
  Serial.print("SafeString result with precision  7: ["); Serial.print(sfStr); Serial.println("]");

  if (snprintf(dStr, 21, "%+20.15f", d) > 20) {  // 20 is (sizeof dStr) - 1 if you don't want to hardcode (and use * in printf format)
    Serial.println("snprintf result with 15 truncated");
    dStr[(sizeof dStr) - 1] = '\0';
  }
  Serial.print("snprintf result with precision 15  : ["); Serial.print(dStr); Serial.println("]");

  if (snprintf(dStr, 21, "%+20.7f", d) > 20) {
    Serial.println("snprintf result with 7 truncated");
    dStr[(sizeof dStr) - 1] = '\0';
  }
  Serial.print("snprintf result with precision  7  : ["); Serial.print(dStr); Serial.println("]");
}

void loop() {}

Serial Monitor (@ 115200 bauds) will show

SafeString result with precision 15: [          +0.0123457]
SafeString result with precision  7: [          +0.0123457]
snprintf result with precision 15  : [  +0.012345678901234]
snprintf result with precision  7  : [          +0.0123457]

On 32 bits Arduino, a double has 64 bits (1 sign bit, 11 exponent bits and 53 - 52 explicitly stored - significand bits).

The 53-bit significand precision leads to up to 17 significant decimal digits precision (2−53 ≈ 1.11 × 10−16). 15 being sure.

Your library truncates arbitrarily at 7 digits (documented in the code but not in the documentation), may be that's something you want to improve to be architecture dependant?

Why would you want to rewrite this utility method every time? That is what libraries, like SafeString, are for. Just getting char[ ] big enough for the maximum possible of result is only a small part of the the work this SafeString print method does.

Fixed in the doc I put up yesterday.
Yes does not handle the full precision of ESP.

My question to you is "Do you have an example sketch where the 15digs of output are needed?".

If you do, I can look at that. Until then just having the library run on ALL the Arduino compatible boards is a big job.

I don't have a need for your library, so I'm fine if you don't support better precision when it's available. (it would probably not be a major rewrite to check sizeof(double) and adjust this code if you get more than 4 (likely will be 8 then))

I see the question as "why would you want to use it every time when it's not needed"...

I gave you a high level scenario where

  • you don't need to check anything. (strcpy, you know the buffer is big enough. You do this yourself in your library)
  • you would want to check prior to calling strcat() (optimal packet based transactions)
  • you go ahead and use strlcat and check afterwards

It's because each of those use cases exist that it's good to have choice and it's good to know about the functions giving you that choice.

This topic was automatically closed 120 days after the last reply. New replies are no longer allowed.