LinuxCNC / linuxcnc

LinuxCNC controls CNC machines. It can drive milling machines, lathes, 3d printers, laser cutters, plasma cutters, robot arms, hexapods, and more.
http://linuxcnc.org/
GNU General Public License v2.0
1.82k stars 1.17k forks source link

Integrate cppcheck in continuous build tests #1574

Open smoe opened 2 years ago

smoe commented 2 years ago

Just a proposal that I would help to implement for 2.9+0.1 if this is considered positive for the project. There are some automated code-checkers that (at times) are surprisingly helpful. A very good stimulus is "cppcheck" - not perfect, but the one or other memory leak or undefined variable or plain error should be spotted with it.

I have tried this in src/hal and found some interesting bits like

src/hal/drivers/opto_ac5.c:432:13: error: Signed integer overflow for expression '1<<(31-i)'. [integerOverflow]
      mask=1<<(31-i);

and the following patch to it

$ vim !$
vim hal/drivers/opto_ac5.c
$ cppcheck hal/drivers/opto_ac5.c
Checking hal/drivers/opto_ac5.c ...
$ git diff !$
git diff hal/drivers/opto_ac5.c
diff --git a/src/hal/drivers/opto_ac5.c b/src/hal/drivers/opto_ac5.c
index 5d81aa5be3..19bdb40ebb 100644
--- a/src/hal/drivers/opto_ac5.c
+++ b/src/hal/drivers/opto_ac5.c
@@ -429,7 +429,7 @@ Device_DigitalOutWrite(void *arg, long period)
                        pDigital = &pboard->port[portnum].io[23];//one before what we want to check
                        for (i = 0;i < 2;i++)
                                {
-                                       mask=1<<(31-i);
+                                       mask = (unsigned int) 1 << (31-i);
                                        pDigital++;

                                        if ( *(pDigital->pValue) ==0 ) {        pins |= mask;       }   

The bug (if this effectively is a bug, anyway) was that integers are signed by default.

SebKuzminsky commented 2 years ago

I'm in favor of increased use of automated code checkers in our CI system.