Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stream class virtual destructor #585

Closed
gicking opened this issue Dec 29, 2024 · 2 comments
Closed

Stream class virtual destructor #585

gicking opened this issue Dec 29, 2024 · 2 comments

Comments

@gicking
Copy link

gicking commented Dec 29, 2024

Hi there,

first of all thanks a lot for your great work and effort! (thumbsup)

I just stumbed accross an issue with the Stream class, when writing a destructor for a derived class. Specifically, the class has a variable pSerial of type Stream*, which is allocated via

this->pSerial = (SoftwareSerial*) (new SoftwareSerial(this->pinRx, this->pinTx, this->inverseLogic));

But when I try to delete it inside the class constructor via

delete (SoftwareSerial*) this->pSerial; or delete static_cast<SoftwareSerial*>(this->pSerial);

I get the warning
warning: deleting object of polymorphic class type 'SoftwareSerial' which has non-virtual destructor might cause undefined behavior [-Wdelete-non-virtual-dtor]

If I understand it correctly, the issue is that class Stream does not have a virtual destructor - or any destructor for that matter, see here.

I understand that using SoftwareSerial dynamically is uncommon, but I have no control over the "correct" use of my library. Therefore I want to avoid memory leaks by releasing the instance again.

To avoid this issue I propose to add a dummy virtual destructor to class Stream. Alternatively, do you know how I could avoid this issue? I tried calling the destructor explicitly

((SoftwareSerial*) this->pSerial)->~SoftwareSerial();

which does compile without warning. However, Co-Pilot advises against it, as it doesn't seem to release allocated memory. Any other ideas?

@matthijskooijman
Copy link
Collaborator

See also arduino/ArduinoCore-API#218 for a similar discussion.

@gicking
Copy link
Author

gicking commented Jan 12, 2025

closed due to parallel discussion arduino/ArduinoCore-API#218

@gicking gicking closed this as completed Jan 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants