Static errors in the C codebase

[AlessandroG09]

I run a check of the C codebase with cppcheck as part of one of my University's course projects. Below, the relevant errors that cppcheck found. Each set of similar errors is enclosed in a quote.

[paparazzi-master/sw/misc/inertial/C/ahrs_quat_fast_ekf_main.c:68]: (error) Array 'afe_P[7][7]' index afe_P[7][1] out of bounds.

I think this might be just a typo? Since I see the matrix is 6 cells for 6 cells.

[paparazzi-master/sw/logalizer/openlog2tlm.c:63]: (error) Resource leak: in
[paparazzi-master/sw/logalizer/openlog2tlm.c:158]: (error) Resource leak: in
[paparazzi-master/sw/logalizer/openlog2tlm.c:158]: (error) Resource leak: out
[paparazzi-master/sw/tools/gps_ublox_conf/ublox_conf.c:287]: (error) Resource leak: in_file

[paparazzi-master/sw/airborne/subsystems/datalink/w5100.c:236]: (error) Array 'ip[1]' accessed at index 1, which is out of bounds.
[paparazzi-master/sw/airborne/subsystems/datalink/w5100.c:237]: (error) Array 'ip[1]' accessed at index 2, which is out of bounds.

Arrays count from zero, so, if the intent was to access that single value stored into ip[], shouldn't it be ip[0] instead of ip[1]?

[paparazzi-master/sw/airborne/firmwares/rotorcraft/main_ap.c:84]: (error) failed to evaluate #if condition, division/modulo by zero
[paparazzi-master/sw/airborne/firmwares/rover/main_ap.c:79]: (error) failed to evaluate #if condition, division/modulo by zero

[paparazzi-master/sw/airborne/arch/lpc21/lpcusb/usbhw_lpc.c:479]: (error) Shifting signed 32-bit value by 31 bits is undefined behaviour
[paparazzi-master/sw/airborne/arch/lpc21/lpcusb/usbhw_lpc.c:528]: (error) Shifting signed 32-bit value by 31 bits is undefined behaviour
[paparazzi-master/sw/airborne/peripherals/ms5611.c:95]: (error) Shifting signed 32-bit value by 31 bits is undefined behaviour
[paparazzi-master/sw/airborne/peripherals/ms5611.c:138]: (error) Shifting signed 32-bit value by 31 bits is undefined behaviour
[paparazzi-master/sw/ext/tlsf/tlsf.c:676]: (error) Shifting signed 32-bit value by 31 bits is undefined behaviour

I hope to be of help reporting this errors.

[podhrmic]

Hi @AlessandroG09 !

[paparazzi-master/sw/misc/inertial/C/ahrs_quat_fast_ekf_main.c:68]: (error) Array 'afe_P[7][7]' index afe_P[7][1] out of bounds.

That definitely looks like a typo. I suspect this code never gets used, so the error didn't show up before.

openlog2tlm and ublox_conf.c errors look very similar to what we have seen in the coverity scan. I believe neither of those is really used anymore, so we didn't bother to fix it (although we probably should).

[paparazzi-master/sw/airborne/subsystems/datalink/w5100.c:236]: (error) Array 'ip[1]' accessed at index 1, which is out of bounds.

That just means the cppcheck doesn't understand how ip is defined:

//the IP address for the shield:
static uint8_t ip[] = { W5100_IP };

Normally W5100_IP is a 4 byte IP address, so I suspect when it wasn't defined the checker assumed 1 byte only.

[paparazzi-master/sw/airborne/firmwares/rotorcraft/main_ap.c:84]: (error) failed to evaluate #if condition, division/modulo by zero

Again, one of the defines wasn't found so the checker assumed 0. These are normally defined in the build process, in the generated makefiles and headers.

[paparazzi-master/sw/airborne/arch/lpc21/lpcusb/usbhw_lpc.c:479]: (error) Shifting signed 32-bit value by 31 bits is undefined behaviour

This might be a problem, although we were using this code in the past and it worked as expected - so perhaps we were lucky with the undefined behavior.

[paparazzi-master/sw/airborne/peripherals/ms5611.c:95]: (error) Shifting signed 32-bit value by 31 bits is undefined behaviour

The values are defined as int_64, so I think the checker here just assumes we have 32-bit ints, but the compiler actually figures out they are 64-bit. Hence a false positive.

[paparazzi-master/sw/ext/tlsf/tlsf.c:676]: (error) Shifting signed 32-bit value by 31 bits is undefined behaviour

Not sure about this one. @gautierhattenberger what do you think?

@AlessandroG09 we had similar false positives previously with coverity scan - I think the main challenge for all those checkers is that paparazzi generates lots of code (mostly Makefiles and headers) dynamically, depending on your configuration. So having a checker test the generated code and flag errors and warnings there would be the most useful. Not sure if that is something you would want to try for your course project?

[OpenUAS]

@AlessandroG09 If I would be your, I'll differentiate between airborne code and Ground Tools only use for specific hardware etc. Thanks any how for you effort in possibly making PPRZ even more robust 👍

[gautierhattenberger]

It might actually be a real possible error when running on a 32bits arch. I need to check but it might be easy to solve with something like 1UL << 31.