Thomas Huth, on Tue 09 Feb 2016 17:14:15 +0100, wrote: > > + return (a.s6_addr[prefix_len / 8] >> (8 - (prefix_len % 8))) > > + == (b.s6_addr[prefix_len / 8] >> (8 - (prefix_len % 8))); > > checkpatch.pl complains here: > > ERROR: return is not a function, parentheses are not required
This is a false positive, there are no outer parentheses here. > There are also some other stylistic problems that checkpatch.pl reports > in this file ... would be nice to fix them. Uh, it seems they are new, I had made a checkpatch run at the time (but that was like several months ago now...). So I'll have to go through it again... > > +static void icmp6_send_echoreply(struct mbuf *m, Slirp *slirp, struct ip6 > > *ip, > > + struct icmp6 *icmp) > > +{ > > + struct mbuf *t = m_get(slirp); > > + t->m_len = sizeof(struct ip6) + ntohs(ip->ip_pl); > > + memcpy(t->m_data, m->m_data, t->m_len); > > + > > + /* IPv6 Packet */ > > + struct ip6 *rip = mtod(t, struct ip6 *); > > Not sure how strictly this is handled in QEMU, but for proper portable > C, variables should be declared at the beginning of a scope, as far as I > know. AIUI, qemu requires C99, doesn't it? Personnally I find it more readable to declare variables where they start mattering instead of far away. > > + m->m_len += ETH_HLEN; > > + m->m_data -= ETH_HLEN; > > + struct ethhdr *eth = mtod(m, struct ethhdr *); > > + m->m_len -= ETH_HLEN; > > + m->m_data += ETH_HLEN; > > Manipulating m_len is not really required here, is it? Indeed, I however preferred to keep it for always keeping consistency, in case mtod starts doing other stuff. > > + /* ICMPv6 Checksum */ > > + t->m_data -= NDPOPT_LINKLAYER_LEN; > > + t->m_data -= ICMP6_NDP_RA_MINLEN; > > + t->m_data -= sizeof(struct ip6); > > I think you could shorten this to: > > t_m_data = rip; That's equivalent, yes. We are here however following the current slirp practice, which explicits what is happening here. I'll also have a closer look at the rest, thanks, Samuel