From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58061) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aTBCS-00032V-AE for qemu-devel@nongnu.org; Tue, 09 Feb 2016 11:31:37 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aTBCP-00033o-3a for qemu-devel@nongnu.org; Tue, 09 Feb 2016 11:31:36 -0500 Received: from mx1.redhat.com ([209.132.183.28]:44728) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aTBCO-00033k-Ry for qemu-devel@nongnu.org; Tue, 09 Feb 2016 11:31:33 -0500 References: <56BA1057.6030409@redhat.com> From: Eric Blake Message-ID: <56BA1462.2050704@redhat.com> Date: Tue, 9 Feb 2016 09:31:30 -0700 MIME-Version: 1.0 In-Reply-To: <56BA1057.6030409@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="tCdDHwlCJbBVtKJSCBlqjkPaif9g6menD" Subject: Re: [Qemu-devel] [PATCHv7 1/9] slirp: Adding IPv6, ICMPv6 Echo and NDP autoconfiguration List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Thomas Huth , Samuel Thibault , qemu-devel@nongnu.org Cc: zhanghailiang , Li Zhijian , Stefan Hajnoczi , Jason Wang , Dave Gilbert , Vasiliy Tolstov , Huangpeng , Gonglei , Jan Kiszka , Yang Hongyang , Guillaume Subiron This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --tCdDHwlCJbBVtKJSCBlqjkPaif9g6menD Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 02/09/2016 09:14 AM, Thomas Huth wrote: > On 08.02.2016 11:28, Samuel Thibault wrote: >> From: Guillaume Subiron >> >> This patch adds the functions needed to handle IPv6 packets. ICMPv6 an= d >> NDP headers are implemented. >> >> +static inline int in6_equal_net(struct in6_addr a, struct in6_addr b,= >> + int prefix_len) >> +{ >> + if (memcmp(&a, &b, prefix_len / 8) !=3D 0) { >> + return 0; >> + } >> + >> + if (prefix_len % 8 =3D=3D 0) { >> + return 1; >> + } >> + >> + return (a.s6_addr[prefix_len / 8] >> (8 - (prefix_len % 8))) >> + =3D=3D (b.s6_addr[prefix_len / 8] >> (8 - (prefix_len % 8)));= >=20 > checkpatch.pl complains here: >=20 > ERROR: return is not a function, parentheses are not required '>>' binds higher than '=3D=3D', so you could write: return a.s6... % 8)) =3D=3D b.s6... % 8)); Make this function return bool, while you are at it. >=20 > There are also some other stylistic problems that checkpatch.pl reports= > in this file ... would be nice to fix them. >=20 >> +} >> + >> +static inline int in6_equal_mach(struct in6_addr a, struct in6_addr b= , >> + int prefix_len) Another candidate for returning bool. >> +static inline void in6_compute_ethaddr(struct in6_addr ip, >> + uint8_t eth[ETH_ALEN]) >> +{ >> + eth[0] =3D 0x52; >> + eth[1] =3D 0x56; >> + memcpy(ð[2], &(ip.s6_addr[16-(ETH_ALEN-2)]), ETH_ALEN-2); >=20 > checkpatch.pl complains about the style here ... and I think you could > also drop the parentheses around "ip.s6_addr[16-(ETH_ALEN-2)]". And remember spaces around both binary '-' >> +++ b/slirp/ip6_icmp.c >> @@ -0,0 +1,350 @@ >> +/* >> + * Copyright (c) 2013 Want to add 2016? >> + * Guillaume Subiron, Yann Bordenave, Serigne Modou Wagne. >> + * >> + * Please read the file COPYRIGHT for the >> + * terms and conditions of the copyright. We don't have a file named 'COPYRIGHT' in the tree. By default you are getting GPLv2+; be explicit if you meant something else. >> + */ >> + >> +#include "slirp.h" >> +#include "ip6_icmp.h" >> +#include "qemu/timer.h" >> +#include "qemu/error-report.h" >> +#include >> +#include New .c files need to include "qemu/osdep.h" first; at which point is pre-included. >> + >> +#define rand_a_b(a, b)\ >> + (rand()%(int)(b-a)+a) Spacing around binary operators. Should we rely on glib for nicer random interval functions? >> +static void icmp6_send_echoreply(struct mbuf *m, Slirp *slirp, struct= ip6 *ip, >> + struct icmp6 *icmp) >> +{ >> + struct mbuf *t =3D m_get(slirp); >> + t->m_len =3D sizeof(struct ip6) + ntohs(ip->ip_pl); >> + memcpy(t->m_data, m->m_data, t->m_len); >> + >> + /* IPv6 Packet */ >> + struct ip6 *rip =3D mtod(t, struct ip6 *); >=20 > 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. It's not portable to C89, but QEMU requires C99 where it is completely portable. However, being portable and being commonly used in the rest of the source tree are two different things. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --tCdDHwlCJbBVtKJSCBlqjkPaif9g6menD Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJWuhRiAAoJEKeha0olJ0NqwaoH/1v3/JdU1cfHCpK0qKNKAdT9 836I1ZopX2jj3pFYpF3Xt0eH4julN/7K4c196Hwj21g/De1QoSi4htO2oEBxVxQe /eDH7+lGn86QipVbHsn59AjCjX0kdv8lGCFsGTJOV0RtA3fBwKjpmMzsenw+Ilww ksffBAeJJFA99L47bL9MM4LlNxHTmkZzlvG5rqzb8xQSHJrNCJj4GaKijUUfmJjg CNOoYMQk1I9DxmScRIJoRlh/ztHdQ71XqUwFlHiht2UecBr+mmip6+4uqeQYSeGV iBETXc2IrgQXKYLC670QFgCf/KHBq9OvvsbTzdOmi8lhtM08LAnhJIphJCRyfyM= =ytmi -----END PGP SIGNATURE----- --tCdDHwlCJbBVtKJSCBlqjkPaif9g6menD--