Tom, in no case can passing & returning string objects by value result in a use after free or memory leak. Pass by value guarantees you'll be operating on a copy as soon as necessary, and all compiler generated copies are guaranteed to be freed by the compiler. What will happen on a call by value: a simple compile without any optimization will allocate a new string instance on the heap in substr(), pass that to SetValue(), let SetValue() create another copy for the assignment, then free the original value and after returning to the calling scope free the heap object. With optimization the compiler will either directly replace the value by the substr() object, or even execute the compare() on the original strings and directly let the substr() result write into the existing value object avoiding malloc/free if possible. In both cases, the compiler knows it does not need to free the temporary / virtual instance because it's become / replaced a persistant instance. Regards, Michael Am 20.08.2018 um 12:36 schrieb 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 _______________________________________________ OvmsDev mailing list OvmsDev@lists.openvehicles.com http://lists.openvehicles.com/mailman/listinfo/ovmsdev
-- Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal Fon 02333 / 833 5735 * Handy 0176 / 206 989 26