Re: nd6: DAD: Remove useless variable, simplify code

[Alexander Bluhm]

On Sun, Nov 27, 2022 at 06:42:48PM +, Klemens Nanni wrote:
> Using a local `duplicate' variable to defer the actual checks by a few
> lines, interleaved with comments (saying the same thing but negated),
> is harder to follow that neccessary.
> 
> Fold the logic and merge comments (remove the last obvious one missing
> a negation) to save 20 LOC.
> 
> Feedback? Objection? OK?

OK bluhm@

> -  * We have transmitted sufficient number of DAD packets.
> +  * We have transmitted enough DAD packets.

Could you leave this line as it is?  Not changing comment makes cvs
blame easier to use.  And "enough" is not better to understand than
"sufficient number of".

nd6: DAD: Remove useless variable, simplify code

[Klemens Nanni]

Using a local `duplicate' variable to defer the actual checks by a few
lines, interleaved with comments (saying the same thing but negated),
is harder to follow that neccessary.

Fold the logic and merge comments (remove the last obvious one missing
a negation) to save 20 LOC.

Feedback? Objection? OK?

diff --git a/sys/netinet6/nd6_nbr.c b/sys/netinet6/nd6_nbr.c
index d8fdc5b9bd6..663795627fc 100644
--- a/sys/netinet6/nd6_nbr.c
+++ b/sys/netinet6/nd6_nbr.c
@@ -1210,29 +1210,14 @@ nd6_dad_timer(void *xifa)
nd6_dad_starttimer(dp, ifa->ifa_ifp->if_nd->retrans);
} else {
/*
-* We have transmitted sufficient number of DAD packets.
-* See what we've got.
+* We have transmitted enough DAD packets.
 */
-   int duplicate;
-
-   duplicate = 0;
-
-   if (dp->dad_na_icount) {
-   duplicate++;
-   }
-
-   if (dp->dad_ns_icount) {
-   /* We've seen NS, means DAD has failed. */
-   duplicate++;
-   }
-
-   if (duplicate) {
+   if (dp->dad_na_icount || dp->dad_ns_icount) {
/* dp will be freed in nd6_dad_duplicated() */
nd6_dad_duplicated(dp);
} else {
/*
 * We are done with DAD.  No NA came, no NS came.
-* duplicated address found.
 */
ia6->ia6_flags &= ~IN6_IFF_TENTATIVE;
 
@@ -1317,12 +1302,10 @@ void
 nd6_dad_ns_input(struct ifaddr *ifa)
 {
struct dadq *dp;
-   int duplicate;
 
if (!ifa)
panic("%s: ifa == NULL", __func__);
 
-   duplicate = 0;
dp = nd6_dad_find(ifa);
if (dp == NULL) {
log(LOG_ERR, "%s: DAD structure not found\n", __func__);
@@ -1333,12 +1316,8 @@ nd6_dad_ns_input(struct ifaddr *ifa)
 * if I'm yet to start DAD, someone else started using this address
 * first.  I have a duplicate and you win.
 */
-   if (dp->dad_ns_ocount == 0)
-   duplicate++;
-
/* XXX more checks for loopback situation - see nd6_dad_timer too */
-
-   if (duplicate) {
+   if (dp->dad_ns_ocount == 0) {
/* dp will be freed in nd6_dad_duplicated() */
nd6_dad_duplicated(dp);
} else {