Re: [PATCH net-next] udp: Neaten and reduce size of compute_score functions

[Eric Dumazet]

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 linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/