`Ntop::checkShutdownWhenDone()` ran the e2e runtime/post test scripts as soon as
`recipients_are_empty()`, which is only one stage of the flow/host alert
pipeline:
checks -> `flowAlertsQueue/hostAlertsQueue` (per-interface)
-> `flowAlertsDequeueLoop`/`hostAlertsDequeueLoop` -> recipients FIFO
-> `recipients.process_notifications` (Lua) -> alert store
Right after a pcap file finishes, flow-end checks have just pushed a whole
batch of alerts into the per-interface queues, which the dequeue loops haven't
moved into the recipients yet - `recipients_are_empty()` is transiently true, so
the runtime script fired before most alerts were persisted.
- `SPSCQueue`: add `isEmpty()` (and constify `isNotEmpty()`).
- `NetworkInterface::alertsQueuesDrained()`: true when the per-interface flow and
host alert queues are both empty.
- `Ntop::alerts_pipeline_drained()`: `alertsQueuesDrained()` on every interface,
plus the internal C->Lua alerts queue, plus `recipients_are_empty()`.
- `checkShutdownWhenDone()`'s guard and `waitRecipientsQueuesDrained()` now poll
`alerts_pipeline_drained()` instead of `recipients_are_empty()` alone, requiring
two consecutive drained reads before running the runtime/post scripts.
Rework the `cppcheck:` recipe so it is usable in practice:
- `--enable=all` -> `--enable=warning,performance,portability`
(drops the `style`/`information`/`unusedFunction` groups, which produced
hundreds of opinion-level hits)
- drop `--force` (was re-analyzing every file once per #ifdef combination;
single files took 40+ min). Analyzes the default build config only.
- add `-j $(shell nproc)` + `--cppcheck-build-dir=.cppcheck-build` for
parallelism and incremental caching; `mkdir -p` the cache dir in the recipe
- suppress checks that are noise for this codebase:
* useInitializationList - micro perf, POD members
* duplInheritedMember - intentional non-virtual name shadowing
(operator[], swap, enqueue, incUses, lua, ...)
* noCopyConstructor / noOperatorEq - the flagged classes
(NetworkInterface, Ntop, Flow, ...) are non-copyable by design and
always held by pointer; the fix cppcheck suggests (add operator=)
is meaningless for them
- .gitignore: ignore the new .cppcheck-build/ cache directory
FlowAlertsLoader::registerRisk (src/FlowAlertsLoader.cpp)
Missing `else`: `alert_to_risk[alert_type.id] = risk` was executed even when
`alert_type.id >= MAX_DEFINED_FLOW_ALERT_TYPE` -> out-of-bounds array write.
The sibling registerAlert() already had the `else`.
Uninitialized ndpi_protocol structs (src/NetworkInterface.cpp x6,
src/LuaEngineInterface.cpp x1)
Local `ndpi_protocol` values were only partially initialized (proto.* set,
but breed/fpc/state/category/custom_category_userdata left indeterminate)
and then passed by value to get_ndpi_full_proto_name() /
get_ndpi_proto_category(). Added `memset(&x, 0, sizeof(x))` at each
declaration, matching the existing convention elsewhere in the tree.
Uninitialized locals
- src/ParserInterface.cpp: `src2dst_direction` was read on the
`flow == NULL` path (getFlow() not called). Initialize to a defined value;
the value is not otherwise observable on that path.
- include/Bitmap.h (toHexString): if `buf_len <= 0` the first loop never
runs and `strlen(buf)` reads uninitialized memory; also `strlen(buf) - 2`
underflows (unsigned) when the string is shorter than 2 bytes. Null-
terminate up front and hoist strlen into a `size_t`, iterate with
`pos + 2 < len`.
src/NetworkInterface.cpp incNumHosts()/decNumHosts()
`if (isShuttingDown() || (!host->isUnicastHost())) return;` - the second
term is always false, the function already returned on `!isUnicastHost()`
a few lines above. Dropped it.
include/NetworkInterface.h getActiveFlowsStats()
Declaration and definition disagreed on the first two parameter *names*
(`stats, status_stats` vs `ndpi_stats, stats`); types matched. Renamed the
header params to match the implementation.
No behavioural change; each argument was already non-negative.
- Interface id (`get_id()` returns int; kept int because 3 call sites test
`== -1`) formatted with %u in redis-key macros -> cast the argument
`(u_int)` at the call site:
include/AutonomousSystem.h, include/Country.h,
include/ObservationPoint.h, include/VLAN.h,
src/BroadcastDomains.cpp (x2), src/HostPools.cpp, src/LocalHost.cpp,
src/LuaEngineInterface.cpp (x3), src/NetworkInterface.cpp (x2),
src/SyslogParserInterface.cpp
- src/HTTPserver.cpp: `%d` with get_http_port()/get_https_port() (u_int)
-> `%u`; the scanf-width builder `"%%%u[^|]..."` fed `int` macros -> `%d`
- src/Flow.cpp: `"%u.%u.%u.%u"` with `int` octets from a `%d` sscanf -> `%d`;
`%u` (arg 13) with get_protocol_category() enum -> cast `(u_int)`
- src/MacManufacturers.cpp: `int _mac[3]` scanned with `%02x`
(wants `unsigned int*`) -> declare `u_int _mac[3]`
- src/StatsManager.cpp: `%lu` with `static_cast<long int>(key)` epoch values
-> `%ld` (respects the explicit signed cast)
- src/Trace.cpp: `"%s.%u"` with loop `int i` / `i + 1` -> `%d`
- src/Prefs.cpp: `%u` in usage() help text fed CONST_DEFAULT_* /
MAX_NUM_* / CLICKHOUSE_MAX_* int macros -> cast `(u_int)`
std::string / std::vector parameters taken by value, changed to `const &`
(declaration and definition kept in sync; none are virtual or overrides, so
callers are source-compatible):
- AggregatedFlowsStats::setInfoKey
- Flow::setL7JSON
- CustomFlowLuaScriptAlert::setAlertMessage
- CustomHostLuaScriptAlert::setAlertMessage, ctor (_msg)
- ScanRealtimeAlert ctor (_alerts, std::vector)
- CustomHostLuaScript::allocAlert (_msg)
- OtherAlertableEntity::getAlertCachedValue / setAlertCacheValue /
triggerAlert / releaseAlert (key, value)
- HostAlert::init and HostAlert(HostCheckID, ...) ctor (_check_name)
- Ntop::luaFlowCheckInfo / luaHostCheckInfo (check_name)
- Ntop::incBlacklisHits (listname)
- Ntop::setLuaCache (key, val)
