Not 8 issues.  8 Files that I found issues in, each with a bunch of them - so around 50 all up.

I did a vimgrep for lines starting with something like 'const char* ident =' ish... 

Yeah, I agree, checking for IsDefined() is a lot less cludgy, so have done that (yay for vim macros). Also the existing code didn't cope with -ve values!

I guess I better fork the project now and submit a pull request!! :) 
//.ichael

On Sun, 2 Oct 2022 at 22:01, Michael Balzer <dexter@expeedo.de> wrote:
Michael,

oh, the dreadful c_str() trap once again… nice find.

The deallocated heap buffer *can* have been reallocated easily, we're running on a preemptive OS and on two CPU cores.

IOW, all of these need to be fixed. Sure there are only 8 of them?

On your is_empty_metric() utility: may be I miss something, but why don't we simply use the metric's IsDefined() method?

Regards,
Michael


Am 02.10.22 um 13:37 schrieb Michael Geddes:
Hi all,

I noticed some behavior on the tpms command I'd copied from the Kia/Kona code, which sent me down a small rabbit hole.
What I was seeing was that the temperature was repeated instead of seeing the pressure and temperature.

Even in vehicle.cpp, we can see code like this:

  const char* range_est = StdMetrics.ms_v_bat_range_est->AsUnitString("-", rangeUnit, 0).c_str();
  if (*range_est != '-')
    writer->printf("Est. range: %s\n", range_est);

My C++ is a bit rusty, however I believe that the 'const char*' won't hold the temporary std::string object returned by 'AsUnitString'.  To do that, I believe we need to assign to a reference (which I know will work).  The problem is that the original code will mostly work (especially in cases like the above), as the deallocated heap memory won't have been reallocated.

  const std::string& range_est = StdMetrics.ms_v_bat_range_est->AsUnitString("-", rangeUnit, 0);
  if (!is_empty_metric(range_est))
    writer->printf("Est. range: %s\n", range_est.c_str());

Where I've defined in vehicle.h the following:
inline bool is_empty_metric(const std::string &measure)
{
  return (measure == "") || (measure[0] == '-');
}

Will the above work functionally?  Would it be better to be:  

return (measure =="") || (measure == "-") 

This occurs in 8 files (outside my new Ioniq 5 files) that I have noted. 

I would make a separate commit for this and do a pull request for it, 

//.ichael


_______________________________________________
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
_______________________________________________
OvmsDev mailing list
OvmsDev@lists.openvehicles.com
http://lists.openvehicles.com/mailman/listinfo/ovmsdev