I am not familiar with this specific case, however, I know a similar one in kernel_netlink.c. DHR are these two cases the same?
I think we grand-fathered such warning(s) in kernel_netlink.c and few other ones too.? The new ones are getting removed my magic coverity fairy at the last minute, just before a release. At that moment it becomes no more new warnings instead thinking merits of such assignments? Indeed such unused assignments are very useful. It would probably avoid many pitfalls. It saved me many times. 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 scan-build report 9 "Dead assignment" and one "Dead increment". Coverity may not agree 100% with scan-build. Coverity can compare previous scan outputs. However, I am yet to find an easy way to compare scan-build outputs and summarize what is new. There is one case where I wonder "not subsequently used" makes clutter. 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 Note the kernel_netlink.c: 1559 #ifdef HAVE_LABELED_IPSEC When labeled ipsec is disabled the warning would move. Dead increment programs/pluto/kernel_netlink.c netlink_add_sa 1556 https://swantest.libreswan.fi/scan-build-2019-02-21-090346-28362-1/report-e02073.html#EndPath So the question that arise is should we add the comment, /*XXX not subsequently used */" more than once? 1556 attr = (struct rtattr *)((char *)attr + attr->rta_len); /* attr not subsequently used */" 1580 attr = (struct rtattr *)((char *)attr + attr->rta_len); /* attr not 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. Also new "Dead assignment" would pop up with some features disabled. I recollect a bit more nasty case when USE_DNSSEC=false. I haven't checked it recently, if it is still there. On Wed, Feb 20, 2019 at 03:56:15PM -0500, D. Hugh Redelmeier wrote: > Andrew has introduced several. Here's one: > > /home/hugh/libreswan/lib/libswan/proposals.c:401:21: warning: Value stored to > 'ps' is never read > lswlogs(log, as); ps = "-"; as = "+"; > ^ ~~~ I suspect at the last minute, just before release, coverity fairy would remove this. And next cycle they get added again:) > I like this useless assignment. As I argued below. How about having a practice? I am sure DHR magic would monitor them, if we don't violently discard them. size += lswlogs(log, sep); /* sep = "-"; sep not subsequently used */ or size += lswlogs(log, sep); /* sep = "-"; sep not subsequently used */ If it is on the same line, the line length fairy would be sad. _______________________________________________ Swan-dev mailing list [email protected] https://lists.libreswan.org/mailman/listinfo/swan-dev
