Arduino (original) (raw)

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

Changes:

remove legacy new behavior, c++ operator new is supposed to never return nullptr
- when exceptions are enabled: a catchable exception is raised
- when exceptions are disabled (defaults): abort is called: caller address is showed for debuggers
unused arduino_new has exactly the same behavior as new (std::nothrow), removing the first.
all source code with new which are checking the returned value are now calling new (std::nothrow) instead
(the others will abort on oom, then reboot)
malloc is untouched: it returns nullptr on oom like new (std::nothrow)

d-a-v changed the title~~(WIP) replace new by new (std::nothrow), remove arduino_new~~ replace new by new (std::nothrow), remove arduino_new

Aug 23, 2020

d-a-v mentioned this pull request

Aug 23, 2020

remove legacy new management

d-a-v changed the title~~replace new by new (std::nothrow), remove arduino_new~~ new w/ OOM now aborts by defaults, or throw an exception

Aug 24, 2020

Can we also have a build option to deliberately keep the old situation?
Or else my code will cause crashes where it is checking the returned pointer as soon as this gets merged.
And right now I cannot change the code to call new(std::nothrow) as it is not yet merged.
I may need to make a macro for it or else my code will become quite messy while I still want to support old and new core libs.

new (std::nothrow) is currently supposed to work. Did you try it ?

@TD-er the whole point of this PR is to clean up the current situation and better conform to the C++ language standard. That means removing the legacy behavior.
Although this is expected to be merged soon, you have ample time to migrate, assuming you really need it.
@d-a-v 's question is valid: new(std::nothrow) is supposed to work now. If it doesn't, We Need To Know.

I have not tried it yet.
I just assumed it wouldn't because this PR implemented it...

I will try to see if it compiles using the current core code and will start making a macro for it.

TD-er added a commit to TD-er/ESPEasy that referenced this pull request

Aug 30, 2020

TD-er mentioned this pull request

Aug 30, 2020

OK, next time I should try it first before panic.
It does compile just fine, and the changes do appear quite easy to do in my code (not that many new calls)

Choose a reason for hiding this comment

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

Looks very nice! Smart move to overload the new(std::nothrow&) operator, I was wondering how you'd fix things.

Thanks for this, finally this is cleaned up!

if (ctx)
delete ctx;
if (sha256)
delete sha256;

Choose a reason for hiding this comment

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

is if (...) redundant here? afaik, delete will be no-op with nullptr

Choose a reason for hiding this comment

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

We don't know which one is nullptr

Choose a reason for hiding this comment

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