Multiple assignments to same variable in MQTTClient.h

[earlephilhower]

First, thanks for the Arduino library. It works quite painlessly on the ESP8266.

However, in src/lib/MQTTClient.h there is a construct in PacketId::getNext() which, while it seems to work OK, is actually undefined by the C spec because you're preincrementing, doing a comparison on, and updating the same variable, "next":

---snip---
class PacketId
{
...
public:
int getNext()
{
return next = (next == MAX_PACKET_ID) ? 1 : ++next;
}
};
---snip---

GCC also complains about this line, so I think there really is something iffy here:
In file included from /home/earle/Arduino/libraries/MQTT/src/MQTTClient.h:14:0,
from /home/earle/Arduino/psychoplug/psychoplug.ino:26:
/home/earle/Arduino/libraries/MQTT/src/lib/MQTTClient.h: In member function 'int MQTT::PacketId::getNext()':
/home/earle/Arduino/libraries/MQTT/src/lib/MQTTClient.h:73:55: warning: operation on '((MQTT::PacketId*)this)->MQTT::PacketId::next' may be undefined [-Wsequence-point]
return next = (next == MAX_PACKET_ID) ? 1 : ++next;

I've unfortunately not traced the code to understand what exactly you're trying to do here, but it feels like splitting this into a couple lines would pay dividends in clarifying things and making sure that no GCC update can bonk it.

Thanks,
-EFP3

[256dpi]

Thanks for raising this. However, this file is part of the underlying Eclipse Paho library. Therefore, we should raise the issue on their repository: https://github.com/eclipse/paho.mqtt.embedded-c. Unfortunately, their repository doesen't get much attention lately. We can try ;).

[earlephilhower]

Got it. I'll double check the code is funky there and make a note in that repo so even if it's not updated, there'll be a record should it barf on some new platform.

[earlephilhower]

Just checked their master/head repo and it's actually fixed there. The preincrement is replaced with a simple "next=next+1". But, of course, the master/1.0.0 tag doesn't have it. :( So should they ever do a release it'll be OK.

https://github.com/eclipse/paho.mqtt.embedded-c/blob/master/MQTTClient/src/MQTTClient.h
...snip...
int getNext()
{
return next = (next == MAX_PACKET_ID) ? 1 : next + 1;
}

[256dpi]

Thanks for the update!