PVS-studio code improvement suggestions

[tonhuisman]

Version: 2.8.2

A recent scan on ESPEasy using PVS-studio revealed a few, most likely low-prio, issues in the IRremoteESP8266 code:

#define _IRREMOTEESP8266_VERSION_VAL(major, minor, patch) \
                                    ((major << 16) | (minor << 8) | (patch))
The macro '_IRREMOTEESP8266_VERSION_VAL' is a dangerous expression. The parameters 'major', 'minor' must be surrounded by parentheses.

As I haven't touched these parts of the sources, I'd like to leave the honor of applying these suggestions (or not) to the core team, as there may be specific reasons for having that coded the way it currently is 😃

Edit: Removed the non-relevant part.

[crankyoldgit]

Thanks!

I've created a PR for hopefully fixing the first problem. It's not a significant issue, but fixing it is better than not.

The second one, I'll have a deeper look at. I'm not sure I understand it fully. (i.e. Suggestions welcome ;-)

[tonhuisman]

not sure I understand it fully

The original definition in the base class has an uint8_t as the last specified argument, where the overridden method in the derived class has a bool argument there (all other args are the same). PVS-studio is giving a warning that this may be unintentional, a.k.a. a bug, but there can be reasons you might want or need it coded like that. Effectively, the bool is mapped by the compiler to an uint8_t (or maybe a byte or char, idk) as they all seem to be synonyms, so when using the derived method and looking at the signature, this could cause confusion.

NB: PVS-studio is rather picky (sometimes even creepy) in its validation process 😃

[crankyoldgit]

Oh thanks. I'll have a look at that with that info in mind. i,e. That was super helpful!

[crankyoldgit]

CWE-670:It is possible a virtual function was overridden incorrectly. See eighth argument of function 'send' in derived class 'GreeYAAHeatpumpIR' and base class 'HeatpumpIR'. …GreeHeatpumpIR.h:90:0
(and the same message for line 99)

Embarisingly, It took me far to long to realise it, but that's not our code it's referencing. 😆

i.e. We have no HeatpumpIR, GreeYAAHeatpumpIR, or even a GreeHeatpumpIR.h.

I think your Princess^H^H^H^H^H^H^H^HCWE is in another Castle^H^H^H^H^H^HRepo.

[tonhuisman]

😊 Indeed I have confused another lib with this one, probably based on the IR in the filename, apologies.

Case closed.

[crankyoldgit]

Leaving this open till the PR is merged.