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

[Paul Wouters]

On Mon, 4 Jan 2021, D. Hugh Redelmeier wrote:

> | 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:

Please don't fight :)

I don't mind the many returns, provided we are not indented too badly so
each line becomes 8 characters at the far end. Using the returns does
make it easier for the reader to step through each check. While Hugh's
fixeup keeps the same ordering, the reader cannot tell whether the order
(eg two checks on one line) was done for layout reasons or for some
specific grouping/logical reason.

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

I agree that this is a disadvantage, but having a huge long clause
with && and || is also cognitively hard.

> |         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 );

Probably the bestest solution would be to refactor this more?

[helmet on] That is the linux Way [/helmet]

Paul