netdev - Re: [PATCH net-next] udp: Neaten and reduce size of compute

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1417495491.5303.11.camel@edumazet-glaptop2.roam.corp.google.com>
Date:	Mon, 01 Dec 2014 20:44:51 -0800
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Joe Perches <joe@...ches.com>
Cc:	netdev <netdev@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next] udp: Neaten and reduce size of compute_score
 functions

On Mon, 2014-12-01 at 19:09 -0800, Joe Perches wrote:
> On Mon, 2014-12-01 at 18:59 -0800, Eric Dumazet wrote:
> > On Mon, 2014-12-01 at 17:39 -0800, Joe Perches wrote:
> > > The compute_score functions are a bit difficult to read.
> > > 
> > > Neaten them a bit to reduce object sizes and make them a
> > > bit more intelligible.
> > > 
> > > Return early to avoid indentation and avoid unnecessary
> > > initializations.
> []
> > > +	if (!(net_eq(sock_net(sk), net) &&
> > > +	      udp_sk(sk)->udp_port_hash == hnum &&
> > > +	      !ipv6_only_sock(sk)))
> > > +		return -1
> > 
> > Or even better :
> > 
> > 
> > 	if (!net_eq(sock_net(sk), net) ||
> > 	    udp_sk(sk)->udp_port_hash != hnum ||
> > 	    ipv6_only_sock(sk))
> > 		return -1;
> 
> Hi Eric.
> 
> Yeah, I thought about it but thought it
> simpler to not change the logic.
> 
> Either way is fine with me.


Your patch does not change the logic at all.

My suggestion is about avoiding double negates which are really hard to
read. This is simple boolean logic rules.

Code was :

if (a && x == y && !ipv6_only_sock(sk))) {
    EXPR1;
} else {
    return -1;
}

So the 'logical' way of negating the condition is actually to not add
double negations and use :

if (!a || x != y || ipv6_only_sock(sk)))
    return -1;
EXPR1;

Instead of the less readable form :

if (!(a && x == y && !ipv6_only_sock(sk))))
    return -1;
EXPR1;

You do not have to ask David permission for this, really.



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html