System crash after using sprintf

Question

My Arduino crashes after using sprintf, To reduce system ram am trying to move away from strings to char and now this happened. Loop not working properly and it's not going to webrequest, spent 2 days to find a solution nothing worked yet. My project is to send temp and humidly to my server i was using strings and after one day of use system very slow to send data to server then i realized stings are the culprit then i changed to char

char ventionoroff[6];
char Foggeronoroff[6] ;
char Exhuastonoroff[6] = "OFF";
char current[4];
char today[10];
bool running = false;
bool isFoggerRunning = false;
bool isExhaustFanRunning = false;
Time t;
void loop() {
 
 t = rtc.getTime();
 hum = dht.readHumidity();
 temp = dht.readTemperature();
 delay(1000);
 
 if (isnan(hum) || isnan(temp)) {
 Serial.println(F("Failed to read from DHT sensor!"));
 return;
 }
 val = digitalRead(powerStatus);
 byte MIN = t.hour;
 byte SEC = t.min;
 
 char timeStamp[20];
 snprintf(today, "%02d:%02d", MIN, SEC);
 Serial.println(today);
if (millis() - lastConnectionTime > postingInterval) {
 Serial.println("reaching here2");
 httpRequest();
 }
 if (val)
 snprintf( current , "YES");
 
 else
 snprintf( current , "NO");
 
 
 if (running == false) {
 snprintf( Foggeronoroff , "WAIT");
 
 } else {
 snprintf( Foggeronoroff , "READY");
 
 }
 
 ventilationfan();
 
void httpRequest() {
Serial.println("shit reached here");
 client.stop();
 char data[10];
 const char phpScript[21] = "";
 const char server[20] = ""; // also change the Host line in httpRequest()
 char strt[6];
 char str[6];
 sprintf(strt, "te=%d", temp);
 strcpy(data, strt);
 strcat (data,"&ti=");
 strcat (data,today);
 strcat (data,"&mot=");
 strcat (data,ventionoroff);
 strcat (data,",");
 strcat (data,Foggeronoroff);
 strcat (data,",");
 strcat (data,current);
 strcat (data,"&mo=");
 strcat (data,Exhuastonoroff);
 sprintf(str, "&hum=%d",hum);
 strcat (data,str);
 
 Serial.println(data);
 int len = strlen(data);
 if (client.connect(server, 80)) {
 client.print("POST ");
 client.print(phpScript);
 client.println(" HTTP/1.1");
 client.println("Host:");
 client.println("User-Agent: Arduino/1.0");
 client.println("Connection: close");
 client.println("Content-Type: application/x-www-form-urlencoded; charset=UTF-8");
 client.print("Content-Length: ");
 client.println(len);
 client.println();
 client.print(data);
 restartnet = 0;
 data[0] = 0;
 Serial.print("data after sucessfull connection");
 Serial.println(data);
 Serial.print("freeMemory after connection");
 Serial.println(freeMemory());
 lastConnectionTime = millis();
 
 } else {
 
 Serial.println(F("Failed"));
 data[0] = 0;
 
 }
}

• chrisl: Do you mean this line? sprintf(strt, "te=%d", temp); - Are you sure, that temp will only have 2 digits? Which type is temp? You didn't include the declaration of it in your question. And why are you doing all the strcat() calls? Why not just using snprintf with a bigger format string, including all the data together?
• Juraj: it doesn't look like data is large enough
• chrisl: And when using snprintf(), shouldn't you also provide the maximum number of characters to write, as this function is meant for that?
• user2037091: @chrisl ttemp is temperature value, i tried to do that but get errors new to sprintf so no idea how to implement to make string
• Juraj: sprintf_P(buff, PSTR("%-15s|%d-%02d-%02d %02d:%02d:%02d|% 5d|% 5d|% 3u|"), eventLongLabels[ix], year(t), month(t), day(t), hour(t), minute(t), second(t), events[ix].value1, events[ix].value2, events[ix].count); cplusplus.com/reference/cstdio/printf

Answer 1

There are quite a few points that could be improved in this code. The main ones have already been shown in the comments:

1. the data buffer was not large enough to hold the whole string, which leads to a buffer overflow, memory corruption and, ultimately, the crash you are experiencing.

2. The second parameter to snprintf() should be the size of the provided buffer.

Here I will add a few points that, although not really defects of this program, could help improved it and save memory.

Here:

char current[4];
// ...
if (val)
 snprintf(current, "YES");
else
 snprintf(current, "NO");

you are not using the current buffer to build a string, but only to store one of two pre-existing strings (the literals "YES" and "NO") that are already stored in memory. Rather than making a copy, you could instead reuse those pre-existing strings, and have current be a pointer to the relevant one:

const char *current;
// ...
if (val)
 current = "YES";
else
 current = "NO";

This only saves two bytes (size of the buffer minus size of the pointer), but this pattern happens multiple times in the program, and the savings add up.

Now, another small optimization. Here:

char strt[6];
sprintf(strt, "te=%d", temp);
strcpy(data, strt);

you do not need the extra strt buffer, as you can write directly into the data buffer. Better yet, you can write everything into the buffer in a single call to snprintf():

snprintf(data, sizeof data,
 "te=%d&ti=%02d:%02d&mot=%s,%s,%s&mo=%s&hum=%d",
 temp, t.hour, t.min, ventionoroff, Foggeronoroff, current,
 Exhuastonoroff, hum);

If you don't like this approach, and would prefer writing the data in separate calls to snprintf(), you can do so without using auxiliary buffers. You have to remember (with a pointer) where within the buffer you finished your previous write, and pass this address to the next snprintf() call. Note that you also have to keep track of the room that is still available, which is easy to do with a second pointer that points to the end of the buffer:

char *p = data; // current writing position
char *end = data + sizeof data; // end of the buffer
p += snprintf(p, end - p, "te=%d", temp);
p += snprintf(p, end - p, "&ti=%s", today);
// ...

• user2037091: snprintf(data, sizeof data, "te=%d&ti=%d&mot=%s,%s,%s&mo=%s&hu=%d", temp, today, ventionoroff, Foggeronoroff, current, Exhuastonoroff, hum); 'today' actualy rtc time , but i get 825 in that char not time , i serial printed the today and i got the time but sending 825 to server dont know where it come
• Edgar Bonet: @user2037091: today was an array of char in the original code. Anyway, I amended the answer in order to print t.hour and t.min instead. This saves an extra auxiliary buffer.