Skip to content

Conversation

@d-a-v
Copy link
Collaborator

@d-a-v d-a-v commented Dec 15, 2021

Unless using it like this:

StreamConstPtr(String("test")).sendAll(elsewhere);

using auto var = StreamConstPtr(String-Temporary) leads to errors because it stores an expected-to-be invalid pointer.

This PR prevents from using temporary String with StreamConstPtr.

(note : StreamConstPtr("test").sendAll(elsewhere) is implicitely using an intermediate String)

edit:

+ missing virtual destructor in Stream::

(warning: deleting object of abstract class type 'Stream' which has non-virtual destructor 
 will cause undefined behavior [-Wdelete-non-virtual-dtor])
(warning: deleting object of abstract class type 'Stream' which has non-virtual destructor will cause undefined behavior [-Wdelete-non-virtual-dtor])
@d-a-v d-a-v added this to the 3.1 milestone Dec 15, 2021
Copy link
Collaborator

@mcspr mcspr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This no longer works

StreamConstPtr test{String("test")}; // String is destroyed next line, can't access it's contents
StreamConstPtr test{"test"}; // implicitly creating String{"test"}, same lifetime as above

This was allowed, but no longer is

StreamConstPtr{"test"}.available(); // ...since we are technically in the same 'expression', still and StreamConstPtr itself is a temporary?
virtual int peek() = 0;

Stream() {}
virtual ~Stream() {}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not mentioned?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

commitlog added in OP

@d-a-v d-a-v merged commit dde2c76 into esp8266:master Jan 3, 2022
@d-a-v d-a-v deleted the streamconstptrNotemporary branch January 3, 2022 12:42
hasenradball pushed a commit to hasenradball/Arduino that referenced this pull request Nov 18, 2024
* StreamConstPtr: prevent from passing a temporary String instance
* unconditionally allow progmem chars
* missing virtual destructor in Stream
(warning: deleting object of abstract class type 'Stream' which has non-virtual destructor will cause undefined behavior [-Wdelete-non-virtual-dtor])
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants