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

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3383 bytes --]

On Tue, Mar 05, 2024 at 11:12:18PM +0100, Stefano Brivio wrote:
> On Sun,  3 Mar 2024 14:51:05 +0100
> Laurent Vivier <lvivier@redhat.com> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]