Hi David,
On Wed, Jun 22, 2016 at 01:00:08PM +0000, David Laight wrote:
> From: Jakub Sitnicki
> > Sent: 22 June 2016 12:34
> ...
> > > - a = attrs[TCP_METRICS_ATTR_ADDR_IPV4];
> > > - if (a) {
> > > + if ((a = attrs[TCP_METRICS_ATTR_ADDR_IPV4])) {
> >
> > Copy the pointer inside the branch?
> >
> > Same gain on indentation while keeping checkpatch happy.
>
> Or as below (hacked from the mails):
>
> a = attrs[TCP_METRICS_ATTR_ADDR_IPV4];
> if (a) {
> if (f.daddr.family && f.daddr.family != AF_INET)
> return 0;
> memcpy(&daddr.data, RTA_DATA(a), 4);
> daddr.bytelen = 4;
> family = AF_INET;
> atype = TCP_METRICS_ATTR_ADDR_IPV4;
> dlen = RTA_PAYLOAD(a);
> } else {
> a = attrs[TCP_METRICS_ATTR_ADDR_IPV6];
> if (!a)
> return 0;
> memcpy(&daddr.data, RTA_DATA(a), 16);
> daddr.bytelen = 16;
> family = AF_INET6;
> atype = TCP_METRICS_ATTR_ADDR_IPV6;
> dlen = RTA_PAYLOAD(a);
> }
Yes, this should work as well and require less code changes overall.
Though I think it's less readable:
| if (a) {
| do a;
| else {
| if (!b)
| return;
| do b;
| }
vs.
| if (a) {
| do a;
| } else if (b) {
| do b;
| } else {
| return;
| }
Don't you think?
Thanks, Phil
