Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork137
Fix pgm_read_ptr in AVR pgmspace.h#118
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Originally, `pgm_read_ptr` used to cast its argument to `const void *`, i.e. a pointer to read-only data. This is incorrect, because the argument is a pointer to a pointer in "flash memory". You cannot cast it to`const void *` and then dereference it, because `void` is not an object type.Also, the pointer itself is read-only, but the data could in theory be mutable. The correct type should be `void *const *`, i.e. a pointer to a read-only pointer to any data.
bblanchon commentedSep 16, 2020
I confirm that this change is necessary to fix the following error: Please merge this pull request. |
matthijskooijman left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Also looks good to me.
One nitpick: I would have a small preference to useconst void ** instead ofvoid *const *, which is a bit easier to read but AFAIU equivalent. This is also what is already used in practice in e.g. the SAMD core:https://github.com/adafruit/ArduinoCore-samd/blob/6a59e8347d01a9a7a51cd71f7981d154d851ba35/cores/arduino/avr/pgmspace.h#L106
bblanchon commentedSep 16, 2020
The core for ESP32 use |
bblanchon commentedSep 16, 2020
The core for ESP8266 uses |
bblanchon commentedSep 16, 2020
bblanchon commentedSep 16, 2020
bblanchon commentedSep 16, 2020
As you see, there are many variants out there. |
matthijskooijman commentedSep 16, 2020
But I'm saying that, AFAIU, |
bblanchon commentedSep 16, 2020
It's not a matter of taste; they are different types:
|
matthijskooijman commentedSep 16, 2020
Hm, seems I was wrong indeed, thanks for insisting. So with that out of the way, the PR as it is now is indeed correct, so I'm for merging it. Note that this also means that the SAMD and Arduino_STM32 versions are indeed also subtly broken (returning |
aentinger commentedOct 7, 2020
Thank you to both@bblanchon and@matthijskooijman 🚀 I'll merge now. |
bblanchon commentedOct 7, 2020
Thanks !!! |
Calling this macro causes the error "'const void*' is not a pointer-to-object type"Related toarduino/ArduinoCore-API#118Fixesbblanchon/ArduinoJson#1947
Originally,
pgm_read_ptrused to cast its argument toconst void *, i.e. a pointer to read-only data.This is incorrect, because the argument is a pointer to a pointer in "flash memory". You cannot cast it to
const void *and then dereference it, becausevoidis not an object type.Also, the pointer itself is read-only, but the data could in theory be mutable.
The correct type should be
void *const *, i.e. a pointer to a read-only pointer to any data.