From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Thu, 10 Oct 2013 10:55:06 +0200 From: Antonio Quartulli Message-ID: <20131010085506.GB576@neomailbox.net> References: <1381347174-3629-1-git-send-email-antonio@meshcoding.com> <1381347174-3629-14-git-send-email-antonio@meshcoding.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="hHWLQfXTYDoKhP50" Content-Disposition: inline In-Reply-To: Subject: Re: [B.A.T.M.A.N.] [PATCH 13/16] batman-adv: add build check macros for packet member offset Reply-To: The list for a Better Approach To Mobile Ad-hoc Networking List-Id: The list for a Better Approach To Mobile Ad-hoc Networking List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: The list for a Better Approach To Mobile Ad-hoc Networking Cc: netdev@vger.kernel.org, Marek Lindner , davem@davemloft.net, Simon Wunderlich --hHWLQfXTYDoKhP50 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Oct 10, 2013 at 09:37:32AM +0100, David Laight wrote: > > Since we removed the __packed from most of the packets, we should > > make sure that the offset generated by the compiler are correct for > > sent/received data. > ... > > + /* compile time checks for struct member offsets */ > > + BUILD_BUG_ON(offsetof(struct batadv_unicast_4addr_packet, src) !=3D 1= 0); > > + BUILD_BUG_ON(offsetof(struct batadv_unicast_packet, dest) !=3D 4); > > + BUILD_BUG_ON(offsetof(struct batadv_unicast_frag_packet, dest) !=3D 4= ); > > + BUILD_BUG_ON(offsetof(struct batadv_unicast_tvlv_packet, dst) !=3D 4); > > + BUILD_BUG_ON(offsetof(struct batadv_icmp_packet, dst) !=3D 4); > > + BUILD_BUG_ON(offsetof(struct batadv_icmp_packet_rr, dst) !=3D 4); >=20 > It is usually enough to check the size of the structures. What if two fields are inverted by mistake in a way that the size of the struct remains the same? The size check would not complain but = the code would not work anymore. We use a "generic" struct to access the initial part of any packet. Therefore these checks are to ensure that the information we are going to a= ccess is really placed at that offset, whatever packet we have. It was not possible to use a common inner struct and so we relied on this t= est to be safe. > Which is also best done in the .h file so it is validated > in all the compilation environments that might be used. >=20 This does not really hurt at the moment because we placed them in main.c wh= ich is a file that is always compiled. But thanks for the suggestion: putting t= hem in the .h file helps to remind the developer to add a new BUILD_ON_BUG when creating a new packet type. Thanks a lot. Regards, --=20 Antonio Quartulli --hHWLQfXTYDoKhP50 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBCAAGBQJSVmtqAAoJEADl0hg6qKeOLFQQAKRJ1dlFC5oc1UcY4aLW7b/K u8K9b9I1xMIADhOngya/DkMKEpi/GoRU5X7V/3fIPw+qnQ+eWi3xiISZzzwsrKTy JgMBTQwCcb5u7+saITiJIMnJbX2bwQQELwRAtf/svaABMc70880iH368k61U2KY6 srdDQghaBvD08UCfJRcRTxe8b+15V6g0uwGp20Hcr92FruZNHBtdMxGFgxnEKp0D 1k/NU+kSx/4Ws8nqiWJ3/wKOf1KSWqnEvRHYKSUtJipy8FcBjD7/2FotR3N70i6w ff/hdk5vhk4EqrDiBs0lIb+pDA1/wrprRnYdf4rX7NFZJVmCXPr4gqZyxvOd4rtm 0k5Rb2z8boFZx0Hp4wYXHC8X6BPVtv2HzLcank+j1iqOux6MlH85XLoRsVfwrzMW RiRRxRRSAQ9poE0ye3t0QrFKz0z539zlAf8+30IY0cvYgiqbmUYlgPVWWxJPQSfF vruUsM6xdGz9i31zj1WD2TRjCXIStCqktGy3sPWHAvq44m531hwG/tOeCSk+sT1t 8LqgY+K1Y4ZBGNtB0xHEKwZDJ5TqIv+XHNLkhud5gXO0Urt9S7dldgWMzLBti7Ia s+MsdEXg1x0gwyqZV+Cb7LNJfp3NOvQksJEK3wo/mtrxBk9jCYI7DCYCzGJtF+M4 VAb60NQTLGV5ij0zY5Lv =65Mn -----END PGP SIGNATURE----- --hHWLQfXTYDoKhP50--