Ensure that _size is properly set in begin #2706
Conversation
libraries/EEPROM/src/EEPROM.cpp
Outdated
| @@ -131,6 +131,7 @@ bool EEPROMClass::begin(size_t size) { | |||
| } | |||
|
|
|||
| _data = new uint8_t[size]; | |||
There was a problem hiding this comment.
Where is the NULL check? new does not always succeed.
Chuck.
There was a problem hiding this comment.
@lbernstone does not like NULL checks :D
There was a problem hiding this comment.
Hey, I didn't even write that bit.
There was a problem hiding this comment.
new does not always succeed.
new doesn't always succeed, but it doesn't return NULL unless it's marked with std::nothrow. It throws an exception on failure.
(Arduino is configured with C++ exception handling enabled, but even if exceptions are disabled the firmware will abort() if new fails without being marked std::nothrow)
gcc will even optimize out a null check on a new result if one is added in the code, as it knows the result can't be NULL. (That's ARM, but Xtensa output will be the same for -Os or higher.)
There was a problem hiding this comment.
@projectgus So, it is a stackdump. Why is begin() a boolean function if it can never return a fail? If I used this function, I would assume because of the declaration I am responsible for error checking.
There was a problem hiding this comment.
I'm not saying you shouldn't check for a failed allocation. I'm saying that a null check on the result of the new operator isn't actually checking anything. There are a number of ways a working out of memory check could be added, the comment hints at some of them.
There was a problem hiding this comment.
(If it was up to me I'd suggest either adding std::nothrow or changing this to malloc().)
There was a problem hiding this comment.
let's go with malloc since that is what is used throughout :)
No description provided.