Re: [PATCH v5 0/9] Add vhost-user support to passt (part 1)

On Tue, Mar 05, 2024 at 11:12:18PM +0100, Stefano Brivio wrote:

On Sun, 3 Mar 2024 14:51:05 +0100 Laurent Vivier wrote:

...

v5: - add a patch to cleanup before change: udp: little cleanup in udp_update_hdrX() to prepare future changes - see detailed v5 history log in each patch

I'm about to apply this, but cppcheck (2.10) tells me (with make cppcheck):

checksum.c:195:33: style: inconclusive: Function 'csum_icmp4' argument 1 names different: declaration 'ih' definition 'icmp4hr'. [funcArgNamesDifferent] void csum_icmp4(struct icmphdr *icmp4hr, const void *payload, size_t len) ^ checksum.h:23:33: note: Function 'csum_icmp4' argument 1 names different: declaration 'ih' definition 'icmp4hr'. void csum_icmp4(struct icmphdr *ih, const void *payload, size_t len); ^ checksum.c:195:33: note: Function 'csum_icmp4' argument 1 names different: declaration 'ih' definition 'icmp4hr'. void csum_icmp4(struct icmphdr *icmp4hr, const void *payload, size_t len) ^ pcap.c:145:47: style: inconclusive: Function 'pcap_iov' argument 2 names different: declaration 'n' definition 'iovcnt'. [funcArgNamesDifferent] void pcap_iov(const struct iovec *iov, size_t iovcnt) ^ pcap.h:12:47: note: Function 'pcap_iov' argument 2 names different: declaration 'n' definition 'iovcnt'. void pcap_iov(const struct iovec *iov, size_t n); ^ pcap.c:145:47: note: Function 'pcap_iov' argument 2 names different: declaration 'n' definition 'iovcnt'. void pcap_iov(const struct iovec *iov, size_t iovcnt) ^ tcp.c:947:45: error: Expression 'tlen,IPPROTO_TCP,(struct in_addr){.s_addr=iph->saddr},(struct in_addr){.s_addr=iph->daddr}' depends on order of evaluation of side effects [unknownEvaluationOrder] (struct in_addr){ .s_addr = iph->saddr }, ^ checksum.c:506:0: style: The function 'csum_iov' is never used. [unusedFunction] uint16_t csum_iov(struct iovec *iov, size_t n, uint32_t init) ^ pcap.c:145:0: style: The function 'pcap_iov' is never used. [unusedFunction] void pcap_iov(const struct iovec *iov, size_t iovcnt) ^ iov.c:150:0: information: Unmatched suppression: unusedFunction [unmatchedSuppression] size_t iov_size(const struct iovec *iov, size_t iov_cnt) ^

they are almost all trivial and I plan to fix them up on merge, but I still have to look into the initialisation in tcp_update_check_tcp4() (tcp.c:947:45).

I'm pretty sure this is a false positive from cppcheck: I think it's misinterpreting this (initializer in struct literal) as an assignment expression. Since there are then two apparent assignments to the same apparent target, we have that warning. The obvious workaround to me is to introduce a couple of struct in_addr typed temporaries, instead of using struct literals. I think that's kind of more readable anyway. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson