[prev in list] [next in list] [prev in thread] [next in thread] 

List:       libreswan-dev
Subject:    Re: [Swan-dev] why there were useless assignments
From:       Paul Wouters <paul () nohats ! ca>
Date:       2019-02-22 14:46:32
Message-ID: alpine.LRH.2.21.1902220940210.32276 () bofh ! nohats ! ca
[Download RAW message or body]

On Thu, 21 Feb 2019, Antony Antony wrote:

> Here is a scan-build output:
> "Dead store Dead increment programs/pluto/kernel_netlink.c netlink_add_sa
> 1580	"
> https://swantest.libreswan.fi/scan-build-2019-02-21-084017-27164-1/report-8916ad.html#EndPath
>
> offending line
> 1580 attr = (struct rtattr *)((char *)attr + attr->rta_len);
>
> And the one that started this thread is:
> Dead assignment	lib/libswan/proposals.c	fmt_proposal	401
> https://swantest.libreswan.fi/scan-build-2019-02-21-084017-27164-1/report-a4e611.html#EndPath

I remember those. It's a list of putting items in a netlink message and
moving the pointer ready for the next item. The last time the pointer is
moved it is not used. Hugh likes that because it ensures extending the
list with one item will not forget to move the pointer.

I had removed them because of coverity warning, and Hugh complained. But
I don't know what we ended up doing. I guess a commented out version of
the line to help the person adding an item to the list was a compromise
of sort?

> The scans would report unused assignment on different lines
> based on what is defined and what is not example lets scan with
> USE_LABELED_IPSEC=no

I think we should kill that define. If we are on a system without
selinux, the labels should just always be empty/not present.

> That would look a bit strange:) Still I thin it is a good practice. I
> imagine in some cases a macro would help too.
> I am playing with one for netlink use, for the code in xfrmi. Without
> freaking out macro police.

A nice way to add netlink payloads would be very nice :)

> size += lswlogs(log, sep); /* sep = "-"; sep not subsequently used */
>
> or
>
> size += lswlogs(log, sep);
> /* sep = "-"; sep not subsequently used */

It will still flag warnings in the compiler / coverity right?

Paul
_______________________________________________
Swan-dev mailing list
Swan-dev@lists.libreswan.org
https://lists.libreswan.org/mailman/listinfo/swan-dev
[prev in list] [next in list] [prev in thread] [next in thread]