[Swan-dev] libswan: ip_endpoint.c: endpoint_eq(): simplify

[D. Hugh Redelmeier]

| From: Andrew Cagney <andrew.cagney at gmail.com>

| Shorter? Yes.
| Simpler? No.

Why doesn't it seem simpler to you?

It looks simpler to me.  Here's why I think that I think so:

- shorter (shorter is often, but not always, simpler)

- a lot less clutter to be understood

- one statement instead of 13 (not counting two that implemented
  dropped functionality) (not counting {}, which are technically
  statements; they were only used to make the code clearer).

- one "return" instead of five

- almost the same tests in almost the same order (the difference isn't 
  important for this discussion)

- the structure (a conjunction of test) is much more obvious in the
  new code.

- the control flow is more obvious and manifest

- Negation is cognitively hard.  (Logically speaking, negation is
  easy.)

  Every use of "if (...) return false" requires the reader to do a
  negation in their head.

  Consider 

	if (lt != rt) {
		return false;
	}

  That requires two negations.  Compare that with with zero for

	lt == rt &&

  There are six ! characters in the old version of the function and
  one in the new.  There are also five "false" constants in the old
  version.  So the old version requires that the reader understand
  eleven negations and the new version only requires one.

Possible downside: it is harder to examine the new version with gdb
because there is only one place for a breakpoint, not 13.  But,
without side-effects, each conjunct of the boolean expression can be
easily examined at that breakpoint.

|         const struct ip_info *lt = address_type(l);
|         const struct ip_info *rt = address_type(r);
| -       if (lt == NULL || rt == NULL) {
| -               /* AF_UNSPEC/NULL are never equal; think NaN */
| -               return false;
| -       }
| -       if (lt != rt) {
| -               return false;
| -       }
| -       if (l->hport != r->hport) {
| -               return false;
| -       }
| -       if (!memeq(&l->bytes, &r->bytes, sizeof(l->bytes))) {
| -               return false;
| -       }
| -       if (l->ipproto != 0 && r->ipproto != 0 &&
| -           l->ipproto != r->ipproto) {
| -               return false;
| -       }
| -       if (l->ipproto == 0 || r->ipproto == 0) {
| -               dbg("endpoint fuzzy ipproto match");
| -       }
| -       return true;
| +       return
| +               lt == rt &&
| +               lt != NULL &&   /* AF_UNSPEC/NULL are never equal */
| +               l->hport == r->hport &&
| +               memeq(&l->bytes, &r->bytes, sizeof(l->bytes)) &&
| +               ( l->ipproto == 0 || r->ipproto == 0 ||
| +                 l->ipproto == r->ipproto );
| 
| 
| On Tue, 29 Dec 2020 at 18:15, D. Hugh Redelmeier
| <hugh at vault.libreswan.fi> wrote:
| >
| > New commits:
| > commit 826c11f4f0834abde7812302239f9ca0b71c9834
| > Author: D. Hugh Redelmeier <hugh at mimosa.com>
| > Date:   Tue Dec 29 18:14:37 2020 -0500
| >
| >     libswan: ip_endpoint.c: endpoint_eq(): simplify
| >
| > _______________________________________________
| > Swan-commit mailing list
| > Swan-commit at lists.libreswan.org
| > https://lists.libreswan.org/mailman/listinfo/swan-commit
| _______________________________________________