Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

StreamConstPtr: disallow passing a String temporary#8410

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
d-a-v merged 5 commits intoesp8266:masterfromd-a-v:streamconstptrNotemporary
Jan 3, 2022

Conversation

@d-a-v
Copy link
Collaborator

@d-a-vd-a-v commentedDec 15, 2021
edited
Loading

Unless using it like this:

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

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

This PR prevents from using temporaryString withStreamConstPtr.

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

edit:

+ missing virtual destructor inStream::

(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-vd-a-v added this to the3.1 milestoneDec 15, 2021
Copy link
Collaborator

@mcsprmcspr 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 contentsStreamConstPtr 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?

virtualintpeek() = 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?

d-a-v reacted with thumbs up emoji
Copy link
CollaboratorAuthor

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-vd-a-v merged commitdde2c76 intoesp8266:masterJan 3, 2022
@d-a-vd-a-v deleted the streamconstptrNotemporary branchJanuary 3, 2022 12:42
hasenradball pushed a commit to hasenradball/Arduino that referenced this pull requestNov 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 freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@mcsprmcsprmcspr approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@d-a-v@mcspr

[8]ページ先頭

©2009-2025 Movatter.jp