avaldebe / PMserial

Arduino library for PM sensors with serial interface
MIT License
53 stars 21 forks source link

SoftwareSerial bug - local variable, dangling pointer #19

Closed heX16 closed 3 years ago

heX16 commented 3 years ago

I look at this code and see an error (explain in cppreference). The variable serial is local (in stack), its lifetime is limited to the area of the constructor. After exiting the constructor, the variable serial will be destroyed, and the variable uart will contain dangling pointer. Copying its address should not prolong the lifetime.

I tried looking for some special case that affects the lifetime of a variable and which will not be "undefined behavior" (0, 1, 2, 3, 4, 5, 6 ). But I couldn't find anything suitable for this code...

If my fears are in vain, please give me a link to a description of this case in the standard or explain how it works.

Problem code:

class SerialPM
{
protected:
  Stream *uart;  // hardware/software serial
....

  SerialPM(PMS sensor, uint8_t rx, uint8_t tx) : pms(sensor)
  {
    SoftwareSerial serial(rx, tx);
    uart = &serial;
    hwSerial = false;
  }
avaldebe commented 3 years ago

I think you're, right. If I remember correctly, I tested this feature with actual HW when I implemented this feature, but I probably hit an undefined behavior of the uC tool-chain.

Will you be interested on submitting a patch/PR?

heX16 commented 3 years ago

Will you be interested on submitting a patch/PR?

Yes! I have an idea to declare "Serial" outside and pass it to pointer initialization (it will be completely analogous to "HardwareSerial"). And make the incoming pointer type "Stream". This will allow you to use any "Serial" and even their non-standard implementations. I'll add code and correct examples of working with "SoftwareSerial". Work with "HardwareSerial" will not change.

avaldebe commented 3 years ago

Will you be interested on submitting a patch/PR?

Yes! I have an idea to declare "Serial" outside and pass it to pointer initialization (it will be completely analogous to "HardwareSerial"). And make the incoming pointer type "Stream". This will allow you to use any "Serial" and even their non-standard implementations. I'll add code and correct examples of working with "SoftwareSerial". Work with "HardwareSerial" will not change.

Awesome!

avaldebe commented 3 years ago

fixed on #20