[Ovmsdev] OvmsMetricString memory lifecycle

[Tom Parker]

I have what I hope is a stupid question.

The additional cellular information code I wrote about yesterday worked 
for most of a day and then the ovms module stopped communicating with 
the modem. I didn't have a data logger connected so I don't know what 
exactly happened. When I got on the console the ovms was going through 
the modem power cycle loop, but never receiving data from the modem.

I tried a few things and then I did a simcom status and it hung after 
printing a few garbage characters instead of the line of information I 
added to the status report yesterday.

This got me to looking at how that memory is handled. I'm doing the 
following in simcom::StandardLineHandler() which copies the similar 
pattern for +CGMR and +ICCID in the same function:

   else if (line.compare(0, 7, "+CPSI: ") == 0)
     {
StandardMetrics.ms_m_net_mdm_cpsi->SetValue(line.substr(7));
     ESP_LOGI(TAG, "CPSI: %s", line.c_str());
     }

I know very little about C++ but from what I gather and reading the code

* std:string.substr() creates an object on the stack pointing to memory 
on the heap
* OvmsMetricString::SetValue(std::string value) copies the object into 
it's internal m_value but it still points to the memory allocated on the 
heap

It's not clear when the heap memory for the std:string created in 
substr() is freed. I think it is considered for freeing at the end of 
simcom::StandardLineHandler as it goes out of scope. But it escapes the 
scope when it is passed to OvmsMetricString::SetValue() so if it were 
freed on function exit, we would have a use after free bug.

Is it possible the compiler notices the reference escapes the scope and 
so doesn't free the string? If it does, then when does it get freed? It 
surely can't be freed when it replaced by the new value inside 
OvmsMetricString::SetValue() -- at first thought, this pattern without 
further hints from the programmer would make it too easy to end up with 
double free bugs or memory leaks. If it isn't freed automatically and it 
isn't freed manually then it looks like we have a memory leak.

Looking at the memory logs that the housekeeping message gives, it 
doesn't look like I have a memory leak, so is there a use after free? If 
there is, why doesn't it cause problems all the time? There are a lot of 
string metrics and at least 2 that use exactly the same pattern I used.

My changes are at 
https://github.com/carrott/Open-Vehicle-Monitoring-System-3/tree/cell-info