Skip to content

Commit

Permalink
[Memory] Fix freeing of allocated strings
Browse files Browse the repository at this point in the history
  • Loading branch information
TD-er committed Jan 28, 2025
1 parent b4947ce commit 21fbe83
Show file tree
Hide file tree
Showing 36 changed files with 87 additions and 58 deletions.
2 changes: 1 addition & 1 deletion src/_C008.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ bool CPlugin_008(CPlugin::Function function, struct EventStruct *event, String&

case CPlugin::Function::CPLUGIN_PROTOCOL_TEMPLATE:
{
event->String1 = String();
free_string(event->String1);
event->String2 = F("demo.php?name=%sysname%&task=%tskname%&valuename=%valname%&value=%value%");
break;
}
Expand Down
2 changes: 1 addition & 1 deletion src/_C010.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ bool CPlugin_010(CPlugin::Function function, struct EventStruct *event, String&

case CPlugin::Function::CPLUGIN_PROTOCOL_TEMPLATE:
{
event->String1 = String();
free_string(event->String1);
event->String2 = F("%sysname%_%tskname%_%valname%=%value%");
break;
}
Expand Down
2 changes: 1 addition & 1 deletion src/_C015.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ bool CPlugin_015(CPlugin::Function function, struct EventStruct *event, String&

if (!isvalid) {
// send empty string to Blynk in case of error
formattedValue = String();
free_string(formattedValue);
}

const String valueName = Cache.getTaskDeviceValueName(event->TaskIndex, x);
Expand Down
4 changes: 2 additions & 2 deletions src/_C016.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,8 @@ bool CPlugin_016(CPlugin::Function function, struct EventStruct *event, String&

case CPlugin::Function::CPLUGIN_PROTOCOL_TEMPLATE:
{
event->String1 = String();
event->String2 = String();
free_string(event->String1);
free_string(event->String2);
break;
}

Expand Down
2 changes: 1 addition & 1 deletion src/_P055_Chiming.ino
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,7 @@ uint8_t Plugin_055_ReadChime(const String& name, String& tokens)
log = strformat(F("Chime: read %s "), fileName.c_str());
}

tokens = String();
free_string(tokens);
fs::File f = tryOpenFile(fileName, "r");

if (f)
Expand Down
2 changes: 1 addition & 1 deletion src/src/Commands/InternalCommands.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ bool do_command_case_check(command_case_data & data,
// The data struct is re-used on each attempt to process an internal command.
// Re-initialize the only two members that may have been altered by a previous call.
data.retval = false;
data.status = String();
free_string(data.status);

if (!checkSourceFlags(data.event->Source, group)) {
data.status = return_incorrect_source();
Expand Down
10 changes: 5 additions & 5 deletions src/src/Controller_struct/C018_data_struct.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ void C018_data_struct::reset() {
delete C018_easySerial;
C018_easySerial = nullptr;
}
cacheDevAddr = String();
cacheHWEUI = String();
cacheSysVer = String();
free_string(cacheDevAddr);
free_string(cacheHWEUI);
free_string(cacheSysVer);
autobaud_success = false;
}

Expand Down Expand Up @@ -157,7 +157,7 @@ bool C018_data_struct::initOTAA(const String& AppEUI, const String& AppKey, cons
if (myLora == nullptr) { return false; }
bool success = myLora->initOTAA(AppEUI, AppKey, DevEUI);

cacheDevAddr = String();
free_string(cacheDevAddr);

C018_logError(F("initOTAA()"));
updateCacheOnInit();
Expand Down Expand Up @@ -334,7 +334,7 @@ void C018_data_struct::updateCacheOnInit() {
cacheDevAddr = myLora->sendRawCommand(F("mac get devaddr"));

if (cacheDevAddr == F("00000000")) {
cacheDevAddr = String();
free_string(cacheDevAddr);
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/src/DataStructs/ExtendedControllerCredentialsStruct.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include "../DataStructs/ExtendedControllerCredentialsStruct.h"

#include "../Helpers/ESPEasy_Storage.h"
#include "../Helpers/StringConverter.h"

#define EXT_CONTR_CRED_USER_OFFSET 0
#define EXT_CONTR_CRED_PASS_OFFSET 1
Expand All @@ -24,7 +25,7 @@ bool ExtendedControllerCredentialsStruct::validateChecksum() const

void ExtendedControllerCredentialsStruct::clear() {
for (size_t i = 0; i < CONTROLLER_MAX * 2; ++i) {
_strings[i] = String();
free_string(_strings[i]);
}
}

Expand Down
7 changes: 4 additions & 3 deletions src/src/DataStructs/LogEntry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,9 @@ bool LogEntry_t::add(const uint8_t loglevel, const String& line)
#endif // ifdef USE_SECOND_HEAP

if (line.length() > LOG_STRUCT_MESSAGE_SIZE - 1) {
_message = std::move(line.substring(0, LOG_STRUCT_MESSAGE_SIZE - 1));
move_special(_message, line.substring(0, LOG_STRUCT_MESSAGE_SIZE - 1));
} else {
reserve_special(_message, line.length());
_message = line;
}
}
Expand All @@ -48,7 +49,7 @@ bool LogEntry_t::add(const uint8_t loglevel, String&& line)
// Need to make a substring, which is a new allocation, on the 2nd heap
HeapSelectIram ephemeral;
#endif // ifdef USE_SECOND_HEAP
_message = move_special(line.substring(0, LOG_STRUCT_MESSAGE_SIZE - 1));
move_special(_message, line.substring(0, LOG_STRUCT_MESSAGE_SIZE - 1));
} else {
move_special(_message, std::move(line));
}
Expand All @@ -59,7 +60,7 @@ bool LogEntry_t::add(const uint8_t loglevel, String&& line)

void LogEntry_t::clear()
{
_message = String();
free_string(_message);
_timestamp = 0;
_loglevel = 0;
}
Expand Down
2 changes: 1 addition & 1 deletion src/src/DataStructs/LogStruct.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
* LogStruct
\*********************************************************************************************/
#ifdef ESP32
#define LOG_STRUCT_MESSAGE_LINES 60
#define LOG_STRUCT_MESSAGE_LINES 120
#else
#ifdef USE_SECOND_HEAP
#define LOG_STRUCT_MESSAGE_LINES 60
Expand Down
3 changes: 2 additions & 1 deletion src/src/DataStructs/Modbus.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "../DataStructs/ControllerSettingsStruct.h"
#include "../ESPEasyCore/ESPEasy_Log.h"
#include "../Helpers/StringConverter.h"


Modbus::Modbus() : ModbusClient(nullptr), errcnt(0), timeout(0),
Expand Down Expand Up @@ -108,7 +109,7 @@ bool Modbus::handle() {
unsigned int RXavailable = 0;

#ifndef BUILD_NO_DEBUG
LogString = String();
free_string(LogString);
#endif
int64_t rxValue = 0;

Expand Down
4 changes: 4 additions & 0 deletions src/src/DataStructs/RulesEventCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ bool RulesEventCache::addLine(const String& line, const String& filename, size_t
# ifdef USE_SECOND_HEAP
HeapSelectDram ephemeral;
# endif // ifdef USE_SECOND_HEAP
#ifdef ESP32
reserve_special(event, event.length());
reserve_special(action, action.length());
#endif

_eventCache.emplace_back(filename, pos, std::move(event), std::move(action));
return true;
Expand Down
2 changes: 1 addition & 1 deletion src/src/DataStructs/Web_StreamingBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ void Web_StreamingBuffer::sendContentBlocking(String& data) {
web_server.sendContent(data);

if (data.length() > (CHUNKED_BUFFER_SIZE + 1)) {
data = String(); // Clear also allocated memory
free_string(data); // Clear also allocated memory
} else {
data.clear();
}
Expand Down
25 changes: 16 additions & 9 deletions src/src/ESPEasyCore/ESPEasy_Log.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
#include "../Helpers/ESPEasy_Storage.h"
#endif

#define UPDATE_LOGLEVEL_ACTIVE_CACHE_INTERVAL 5000

/********************************************************************************************\
Init critical variables for logging (important during initial factory reset stuff )
\*********************************************************************************************/
Expand Down Expand Up @@ -124,6 +126,13 @@ bool loglevelActiveFor(uint8_t logLevel) {
return false;
}
#endif
static uint32_t lastUpdateLogLevelCache = 0;
if (lastUpdateLogLevelCache == 0 ||
timePassedSince(lastUpdateLogLevelCache) > UPDATE_LOGLEVEL_ACTIVE_CACHE_INTERVAL)
{
lastUpdateLogLevelCache = millis();
updateLogLevelCache();
}
return logLevel <= highest_active_log_level;
}

Expand All @@ -143,15 +152,13 @@ uint8_t getSerialLogLevel() {
}

uint8_t getWebLogLevel() {
uint8_t logLevelSettings = 0;
if (Logging.logActiveRead()) {
logLevelSettings = Settings.WebLogLevel;
} else {
if (Settings.WebLogLevel != 0) {
updateLogLevelCache();
}
return Settings.WebLogLevel;
}
if (Settings.WebLogLevel != LOG_LEVEL_NONE) {
updateLogLevelCache();
}
return logLevelSettings;
return LOG_LEVEL_NONE;
}

bool loglevelActiveFor(uint8_t destination, uint8_t logLevel) {
Expand Down Expand Up @@ -245,7 +252,7 @@ void addLog(uint8_t logLevel, const char *line)
}
}
#else
if (!copy.reserve(strlen_P((PGM_P)line))) {
if (!reserve_special(copy, strlen_P((PGM_P)line))) {
return;
}
copy = line;
Expand Down Expand Up @@ -353,5 +360,5 @@ void addToLogMove(uint8_t logLevel, String&& string)
Logging.add(logLevel, std::move(string));
}
// Make sure the string may no longer keep up memory
string = String();
free_string(string);
}
6 changes: 3 additions & 3 deletions src/src/Globals/RamTracker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ RamTracker::RamTracker(void) {
writePtr = 0;

for (int i = 0; i < TRACES; i++) {
traces[i] = String();
free_string(traces[i]);
tracesMemory[i] = 0xffffffff; // init with best case memory values, so they get replaced if memory goes lower
}

Expand All @@ -172,7 +172,7 @@ void RamTracker::registerRamState(const String& s) { // store function
int bestCase = bestCaseTrace(); // find best case memory trace

if (ESP.getFreeHeap() < tracesMemory[bestCase]) { // compare to current memory value
traces[bestCase] = String();
free_string(traces[bestCase]);
readPtr = writePtr + 1; // read out buffer, oldest value first

if (readPtr >= TRACEENTRIES) {
Expand Down Expand Up @@ -210,7 +210,7 @@ void RamTracker::getTraceBuffer() {
retval += ' ';
retval += traces[i];
addLogMove(LOG_LEVEL_DEBUG_DEV, retval);
retval = String();
retval.clear();
}
}
#endif // ifndef BUILD_NO_DEBUG
Expand Down
3 changes: 2 additions & 1 deletion src/src/Helpers/Audio.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "../Globals/RamTracker.h"
#include "../Helpers/Hardware_GPIO.h"
#include "../Helpers/Hardware_PWM.h"
#include "../Helpers/StringConverter.h"


/********************************************************************************************\
Expand Down Expand Up @@ -53,7 +54,7 @@ void clear_rtttl_melody() {
# endif // if FEATURE_RTTTL_EVENTS
}

rtttlMelody = String();
free_string(rtttlMelody);
}

void set_rtttl_melody(String& melody) {
Expand Down
2 changes: 1 addition & 1 deletion src/src/Helpers/ESPEasy_checks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ String ReportOffsetErrorInStruct(const String& structname, size_t offset) {
* Not a member function to be able to use the F-macro
\*********************************************************************************************/
bool SettingsCheck(String& error) {
error = String();
free_string(error);
#ifndef LIMIT_BUILD_SIZE
#ifdef esp8266
size_t offset = offsetof(SettingsStruct, ResetFactoryDefaultPreference);
Expand Down
6 changes: 5 additions & 1 deletion src/src/Helpers/Memory.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,8 @@ void *special_realloc(void *ptr, size_t size);
void *special_calloc(size_t num, size_t size);


bool String_reserve_special(String& str, size_t size);
// Perform a special memory allocate for the buffer pointer in the String object
// Allocation is tried either on 2nd heap (ESP8266) or PSRAM (ESP32)
// when possible.
bool String_reserve_special(String& str, size_t size);

2 changes: 1 addition & 1 deletion src/src/Helpers/Modbus_RTU.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ void ModbusRTU_struct::reset() {
delete easySerial;
easySerial = nullptr;
}
detected_device_description = String();
free_string(detected_device_description);

for (int i = 0; i < 8; ++i) {
_sendframe[i] = 0;
Expand Down
10 changes: 8 additions & 2 deletions src/src/Helpers/StringConverter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ void move_special(String& dest, String&& source) {
HeapSelectIram ephemeral;
if (dest.reserve(source.length())) {
dest = source;
source = String();
free_string(source);
return;
}
// Could not allocate on 2nd heap, so just move existing string
Expand All @@ -106,6 +106,12 @@ bool reserve_special(String& str, size_t size) {
return String_reserve_special(str, size);
}

void free_string(String& str) {
// This is a call specifically tailored to what is done in:
// void String::move(String &rhs)

String tmp(std::move(str));
}

/********************************************************************************************\
Format string using vsnprintf
Expand Down Expand Up @@ -1515,7 +1521,7 @@ bool GetArgv(const char *string, String& argvString, unsigned int argc, char sep
int pos_begin, pos_end;
bool hasArgument = GetArgvBeginEnd(string, argc, pos_begin, pos_end, separator);

argvString = String();
free_string(argvString);

if (!hasArgument) { return false; }

Expand Down
4 changes: 4 additions & 0 deletions src/src/Helpers/StringConverter.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ String move_special(String&& source);
// Try to reserve on the heap with the most space available
bool reserve_special(String& str, size_t size);

// Arduino String does not have a function to de-allocate its internal buffer
// This is a special trick to de-allocate its internal buffer and thus free up memory.
void free_string(String& str);

/*
template <typename T>
bool equals(const String& str, const T &val) {
Expand Down
4 changes: 2 additions & 2 deletions src/src/Helpers/StringParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ void transformValue(
}

if (equals(value, '0')) {
value = String();
free_string(value);
} else {
const int valueLength = value.length();

Expand Down Expand Up @@ -769,7 +769,7 @@ bool findNextDevValNameInString(const String& input, int& startpos, int& endpos,
move_special(format, valueName.substring(hashpos + 1));
move_special(valueName, valueName.substring(0, hashpos));
} else {
format = String();
free_string(format);
}
deviceName.toLowerCase();
valueName.toLowerCase();
Expand Down
2 changes: 1 addition & 1 deletion src/src/Helpers/WebServer_commandHelper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ HandledWebCommand_result handle_command_from_web(EventValueSource::Enum source,

bool handledCmd = false;
bool sendOK = false;
printWebString = String();
free_string(printWebString);
printToWeb = false;
printToWebJSON = false;

Expand Down
3 changes: 2 additions & 1 deletion src/src/Helpers/_CPlugin_Helper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ bool safeReadStringUntil(Stream & input,
const unsigned long timer = start + timeout;
unsigned long backgroundtasks_timer = start + 10;

str = String();
// FIXME TD-er: Should this also de-allocate internal buffer?
str.clear();

do {
// read character
Expand Down
2 changes: 1 addition & 1 deletion src/src/PluginStructs/P020_data_struct.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ void P020_Task::discardClientIn() {
}

void P020_Task::clearBuffer() {
serial_buffer = String();
free_string(serial_buffer);
_maxDataGramSize = serial_processing == P020_Events::P1WiFiGateway
? P020_P1_DATAGRAM_MAX_SIZE
: P020_DATAGRAM_MAX_SIZE;
Expand Down
Loading

0 comments on commit 21fbe83

Please sign in to comment.