search

tempesta-tech / tempesta

All-in-one solution for high performance web content delivery and advanced protection against DDoS and web attacks
https://tempesta-tech.com/
GNU General Public License v2.0
618 stars 103 forks source link

Coverity Scan static code analisys on pull request checks #1528

Open krizhanovsky opened 3 years ago

krizhanovsky commented 3 years ago

1526 fixes a dumb bug with signed/unsigned variable. Both the bugs could be found by a static analysis tool, which we run only from time to time due to massive false positives.

The tool integration as a pull request check will make the process less painful since there will be less work on each pull request.

The pull request by the issue must fix all alerts for the current code.

Also consider to compile Tempesta with GCC and/or Clang static analyzers.

snizovtsev commented 3 years ago

In addition to general-purpose analyzer, kernel has its own set of coccinelle scripts for checking common kernel-specific mistakes. They could be launched by make coccicheck M=$TEMPESTA_DIR.

Currently it shows some issues ``` ../tempesta//db/core/tdb.h:220:5-24: atomic_dec_and_test variation before object free at line 221. ../tempesta//fw/sock_srv.c:1191:19-26: WARNING opportunity for kmemdup ../tempesta//fw/apm.c:1496:8-15: WARNING opportunity for kmemdup ../tempesta//tls/x509_crl.c:323:5-12: WARNING opportunity for kmemdup warning: line 134: should no_llseek be a metavariable? warning: line 141: should noop_llseek be a metavariable? warning: line 223: should nonseekable_open be a metavariable? warning: line 290: should nonseekable_open be a metavariable? warning: line 338: should nonseekable_open be a metavariable? ../tempesta//fw/sock_clnt.c:743:2-20: WARNING: NULL check before some freeing functions is not needed. ../tempesta//fw/sock_clnt.c:745:2-20: WARNING: NULL check before some freeing functions is not needed. ../tempesta//fw/sock_srv.c:1848:2-7: WARNING: NULL check before some freeing functions is not needed. ../tempesta//fw/sock_srv.c:1852:2-7: WARNING: NULL check before some freeing functions is not needed. ../tempesta//fw/sock_srv.c:1542:15-18: ERROR: invalid reference to the index variable of the iterator on line 1534 ../tempesta//tls/tls_ticket.c:253:1-7: preceding lock on line 235 ../tempesta//tls/tls_ticket.c:253:1-7: preceding lock on line 242 ../tempesta//fw/hpack.c:132:49-50: WARNING: Use ARRAY_SIZE ../tempesta//fw/t/fuzzer.c:197:29-30: WARNING: Use ARRAY_SIZE ../tempesta//fw/t/fuzzer.c:157:39-40: WARNING: Use ARRAY_SIZE ../tempesta//fw/t/fuzzer.c:278:15-16: WARNING: Use ARRAY_SIZE ../tempesta//fw/t/fuzzer.c:280:16-17: WARNING: Use ARRAY_SIZE ../tempesta//fw/t/fuzzer.c:282:17-18: WARNING: Use ARRAY_SIZE ../tempesta//fw/t/fuzzer.c:284:18-19: WARNING: Use ARRAY_SIZE ../tempesta//fw/t/fuzzer.c:286:23-24: WARNING: Use ARRAY_SIZE ../tempesta//fw/t/fuzzer.c:288:17-18: WARNING: Use ARRAY_SIZE ../tempesta//fw/t/fuzzer.c:290:17-18: WARNING: Use ARRAY_SIZE ../tempesta//fw/t/fuzzer.c:292:15-16: WARNING: Use ARRAY_SIZE ../tempesta//fw/t/fuzzer.c:294:17-18: WARNING: Use ARRAY_SIZE ../tempesta//fw/t/fuzzer.c:296:17-18: WARNING: Use ARRAY_SIZE ../tempesta//fw/t/fuzzer.c:298:21-22: WARNING: Use ARRAY_SIZE ../tempesta//fw/t/fuzzer.c:300:20-21: WARNING: Use ARRAY_SIZE ../tempesta//fw/t/fuzzer.c:303:26-27: WARNING: Use ARRAY_SIZE ../tempesta//fw/t/fuzzer.c:305:15-16: WARNING: Use ARRAY_SIZE ../tempesta//fw/t/fuzzer.c:307:24-25: WARNING: Use ARRAY_SIZE ../tempesta//fw/t/fuzzer.c:309:24-25: WARNING: Use ARRAY_SIZE ../tempesta//fw/t/fuzzer.c:311:22-23: WARNING: Use ARRAY_SIZE ../tempesta//fw/t/fuzzer.c:313:15-16: WARNING: Use ARRAY_SIZE ../tempesta//fw/t/fuzzer.c:315:19-20: WARNING: Use ARRAY_SIZE ../tempesta//fw/t/fuzzer.c:317:13-14: WARNING: Use ARRAY_SIZE ../tempesta//fw/t/fuzzer.c:319:15-16: WARNING: Use ARRAY_SIZE ../tempesta//fw/t/fuzzer.c:321:22-23: WARNING: Use ARRAY_SIZE ../tempesta//fw/t/fuzzer.c:323:16-17: WARNING: Use ARRAY_SIZE ../tempesta//fw/hpack/hgen.c:297:33-34: WARNING: Use ARRAY_SIZE ../tempesta//fw/cfg.c:735:42-47: WARNING: conversion to bool not needed here ../tempesta//fw/sock_srv.c:1251:1-25: WARNING: Assignment of 0/1 to bool variable ../tempesta//fw/sock_srv.c:1147:1-25: WARNING: Assignment of 0/1 to bool variable ../tempesta//fw/sock_srv.c:1132:1-25: WARNING: Assignment of 0/1 to bool variable ../tempesta//fw/sock_srv.c:1104:1-25: WARNING: Assignment of 0/1 to bool variable ../tempesta//fw/sock_srv.c:1183:1-25: WARNING: Assignment of 0/1 to bool variable ../tempesta//fw/sock_srv.c:1833:1-21: WARNING: Assignment of 0/1 to bool variable ../tempesta/fw/server.h:125:8-12: WARNING use flexible-array member instead (https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays) ../tempesta/fw/http_sched_hash.c:203:2-3: Unneeded semicolon ../tempesta/fw/t/unit/user_space/percentiles.c:159:50-51: Unneeded semicolon ../tempesta/fw/http_msg.c:755:2-3: Unneeded semicolon ../tempesta/fw/http_parser.c:800:2-3: Unneeded semicolon ```
krizhanovsky commented 3 weeks ago

This is seems crucial for the project stability: I reckon this bug https://github.com/tempesta-tech/tempesta/pull/2208/commits/e22a45ddf2782710286f5f427b2b9f99b3f0e417 , which crashed our recent setup, could be found by a static analysis