knolleary / pubsubclient

A client library for the Arduino Ethernet Shield that provides support for MQTT.
http://pubsubclient.knolleary.net/
MIT License
3.83k stars 1.47k forks source link

warning: comparison of integer expressions of different signedness #895

Open SteveRMann opened 3 years ago

SteveRMann commented 3 years ago

I know it's "just a warning", but it is so easy to fix.

Complete warning:

C:\Users\steve\Documents\Arduino\libraries\PubSubClient\src\PubSubClient.cpp:523:16: warning: comparison of integer expressions of different signedness: 'unsigned int' and 'int' [-Wsign-compare]
  523 |     return (rc == expectedLength);
      |             ~~~^~~~~~~~~~~~~~~~~

Solution: In PubSubClient.cpp, at line 481, change unsigned int rc to uint16_t rc

m1cje commented 2 years ago

unsigned int rc is a 32 bit value, changing it to uint16_t may have unforeseen consequences, better to change expectedLength to be unsigned as it's unlikely to ever be negative

stefan123t commented 2 years ago

The suggested simple fix by @m1cje to declare unsigned int expectedLength in

https://github.com/knolleary/pubsubclient/blob/2d228f2f862a95846c65a8518c79f48dfc8f188c/src/PubSubClient.cpp#L487

is obviously valid as:

https://github.com/knolleary/pubsubclient/blob/2d228f2f862a95846c65a8518c79f48dfc8f188c/src/PubSubClient.cpp#L521-L523

All of llen, tlen and plength are uint8/16_t or unsigned int either. Hence expectedLength cannot become negative at all.

Thanks for providing and maintaining this library in the first place!