sprintf_P, PROGMEM and __FlashStringHelper

I'm trying to add logging functionality to a sketch but I'm not able to find out how to make the code work. I'v googled it and tried different things but all I get is either a controller restart, a loooooong line with garbage or total freeze.

So how do I use sprintf_P to format a text string?

Some code:

const int formatBufferSize = 96;

const char logprefix01[] PROGMEM = "TMP";
const char logprefix02[] PROGMEM = "WPH";
const char logprefix03[] PROGMEM = "CFG";
const char logprefix04[] PROGMEM = "EEP";
const char logprefix05[] PROGMEM = "MAP";
const char logprefix06[] PROGMEM = "BAT";
const char logprefix07[] PROGMEM = "CHG";
const char logprefix08[] PROGMEM = "POW";
const char logprefix09[] PROGMEM = "ACT";
const char logprefix10[] PROGMEM = "LOG";

const char* const logPrefix[logtypeCount] = {
  logprefix01,
  logprefix02,
  logprefix03,
  logprefix04,
  logprefix05,
  logprefix06,
  logprefix07,
  logprefix08,
  logprefix09,
  logprefix10,
};

bool Logger::log(const int type, const char *msg)
{
  char    str[formatBufferSize];

  // This lines casing the problem
  sprintf_P(str, PSTR("%lu;%s;%ld;%s"), m_sequenceNumber, logPrefix[type], mega.getUnixtime(), msg);

  return write(str);
}

bool Logger::write(const char *line)
{
  // For debugging, just print the line.
  Serial.print(F("LOGG:"));
  Serial.println(line);
  ++m_sequenceNumber;

  return true;
}

void setup()
{
  // code...
  logger.log(9, F("Startup"));
  // more code...
}

Use %S instead of %s for strings in PROGMEM. Please see the documentation.

You probably shouldn't be using sprintf_P, it's a buffer overflow waiting to happen. Use snprintf_P and make sure the strings are always properly terminated.

Pieter

PieterP:
Use %S instead of %s for strings in PROGMEM. Please see the documentation.

You probably shouldn't be using sprintf_P, it's a buffer overflow waiting to happen. Use snprintf_P and make sure the strings are always properly terminated.

Pieter

Thanks for the snprintf_P.

Tried this:

snprintf_P(str, formatBufferSize, PSTR("%lu;%S;%lu;%S"), m_sequenceNumber, logPrefix[type], mega.getUnixtime(), (__FlashStringHelper*)msg);

but still problems. This give me:

22:44:19.816 -> LOGG:37224448;]⸮@⸮⸮
22:44:19.816 -> ⸮⸮⸮⸮⸮q?⸮⸮
22:44:19.856 -> ⸮R⸮⸮⸮⸮⸮⸮⸮⸮⸮⸮	⸮⸮oˁ⸮⸮q;121462809;b⸮⸮i⸮z⸮⸮/⸮⸮⸮⸮

The following code correctly prints 987654321;<STRING-1>;123456789;<STRING-2> when executed on an Arduino Leonardo:

void setup() {
  Serial.begin(115200);
  while(!Serial);
  char buf[128]{};
  static const char pstr1[] PROGMEM = "<STRING-1>";
  static const char pstr2[] PROGMEM = "<STRING-2>";
  unsigned long ul1 = 987654321;
  unsigned long ul2 = 123456789;
  snprintf_P(buf, sizeof(buf), PSTR("%lu;%S;%lu;%S"), ul1, pstr1, ul2, pstr2);
  Serial.println(buf);
}


void loop() {}

Are your snprintf_P arguments the correct type? C-style variadic functions have no type checking whatsoever, so if you specify %lu and then pass an int instead of unsigned long, bad things happen …

This version works. I had to add some to get it to compile.

const int formatBufferSize = 96;
const int logtypeCount = 10;
unsigned long m_sequenceNumber = 0;


const char logprefix01[] PROGMEM = "TMP";
const char logprefix02[] PROGMEM = "WPH";
const char logprefix03[] PROGMEM = "CFG";
const char logprefix04[] PROGMEM = "EEP";
const char logprefix05[] PROGMEM = "MAP";
const char logprefix06[] PROGMEM = "BAT";
const char logprefix07[] PROGMEM = "CHG";
const char logprefix08[] PROGMEM = "POW";
const char logprefix09[] PROGMEM = "ACT";
const char logprefix10[] PROGMEM = "LOG";


const char* const logPrefix[logtypeCount] =
{
  logprefix01,
  logprefix02,
  logprefix03,
  logprefix04,
  logprefix05,
  logprefix06,
  logprefix07,
  logprefix08,
  logprefix09,
  logprefix10,
};


class Logger
{
  public:
  Logger() {};
  bool log(const int type, const char *msg);
  bool write(const char *line);
};


bool Logger::log(const int type, const char *msg)
{
  char    str[formatBufferSize];


  // This lines casing the problem
  snprintf_P(str, formatBufferSize, PSTR("%lu;%S;%S"), m_sequenceNumber, logPrefix[type], msg);
  return write(str);
}


bool Logger::write(const char *line)
{
  // For debugging, just print the line.
  Serial.print(F("LOGG:"));
  Serial.println(line);
  ++m_sequenceNumber;


  return true;
}


Logger logger;


void setup()
{
  Serial.begin(115200);
  
  // code...
  logger.log(9, (const char *) F("Startup"));
  // more code...
}


void loop() {}

Instead of casting to (const char *) in the function call, declare the argument as const __FlashStringHelper * in the function header, that way you can overload the function to also take a string in ram.

const int formatBufferSize = 96;
const int logtypeCount = 10;
unsigned long m_sequenceNumber = 0;

const char logprefix01[] PROGMEM = "TMP";
const char logprefix02[] PROGMEM = "WPH";
const char logprefix03[] PROGMEM = "CFG";
const char logprefix04[] PROGMEM = "EEP";
const char logprefix05[] PROGMEM = "MAP";
const char logprefix06[] PROGMEM = "BAT";
const char logprefix07[] PROGMEM = "CHG";
const char logprefix08[] PROGMEM = "POW";
const char logprefix09[] PROGMEM = "ACT";
const char logprefix10[] PROGMEM = "LOG";

const char* const logPrefix[logtypeCount] =
{
  logprefix01,
  logprefix02,
  logprefix03,
  logprefix04,
  logprefix05,
  logprefix06,
  logprefix07,
  logprefix08,
  logprefix09,
  logprefix10,
};

class Logger
{
  public:
    Logger() {};
    bool log(const int type, const char *msg);
    bool log(const int type, const __FlashStringHelper *msg);
    bool write(const char *line);
};

bool Logger::log(const int type, const char *msg)
{
  char    str[formatBufferSize];

  // This lines casing the problem
  snprintf_P(str, formatBufferSize, PSTR("%lu;%S;%s"), m_sequenceNumber, logPrefix[type], msg);
  return write(str);
}

bool Logger::log(const int type, const __FlashStringHelper *msg)
{
  char    str[formatBufferSize];

  // This lines casing the problem
  snprintf_P(str, formatBufferSize, PSTR("%lu;%S;%S"), m_sequenceNumber, logPrefix[type], msg);
  return write(str);
}

bool Logger::write(const char *line)
{
  // For debugging, just print the line.
  Serial.print(F("LOGG:"));
  Serial.println(line);
  ++m_sequenceNumber;

  return true;
}

Logger logger;

void setup()
{
  Serial.begin(115200);

  // code...
  logger.log(9, F("Startup"));
  logger.log(5, "battery");
  char charge[] = "charge";
  logger.log(6, charge);
  
  // more code...
}


void loop() {}

A lot of thanks to both johnwasser and david_2018. :slight_smile:

It worked (after I removed another bug. Noticed that the m_sequenceNumber was incidentally declared as a uint16_t).

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