search

adafruit / Adafruit_MQTT_Library

Arduino library for MQTT support
MIT License
571 stars 292 forks source link

Removed all occurences of pgm_read_byte #172

Open eutill opened 4 years ago

eutill commented 4 years ago

Hello Adafruit,

after some deep diving in this library's source code that you are so kind to provide and which, by the way, does an awesome job together with your FONA module and Adafruit IO, I found an remnant of the times where user authentication data were still saved in PROGMEM. Unfortunately, these 7 occurences of pgm_read_byte that are still there have gone unnoticed until today. They only cause problems if one tries to comment out #define MQTT_DEBUG. I won't go into detail but I think that these small adjustments will help some people as this will potentially solve issue #54.

I hope that this library is still being maintained. Thank you! eutill

eutill commented 3 years ago

Dear developers at Adafruit, is this library still maintained? You may want to take a look at my pull request as it solves issue #54. I think it is an important issue to address because it prevents the library from working (it completely breaks the MQTT authentication process) when one tries to comment out #define MQTT_DEBUG.

If you're having problems reproducing the problem (although it should be clear that pgm_read_byte is not needed anymore since user authentication data aren't stored in PROGMEM anymore), please let me know and I will provide a better explanation.

Thanks, eutill

brentru commented 3 years ago

@ladyada Is this OK to merge, do we still want to support pgm_read_byte for any reason?

ladyada commented 3 years ago

yeah can remove, its not a PROGMEM, its const char https://github.com/adafruit/Adafruit_MQTT_Library/blob/master/Adafruit_MQTT.h#L249

eutill commented 3 years ago

@brentru I changed the null termination checks back to the usual form. You can go ahead now.