On Thu, Apr 29, 2010 at 11:57:32PM +0200, Joakim Tjernlund wrote: > > Ondrej Zajicek <santi...@crfreenet.org> wrote on 2010/04/29 23:15:22: > > > > On Thu, Apr 29, 2010 at 11:03:32PM +0200, Joakim Tjernlund wrote: > > > > > > Ondrej, this looks buggy: > > > > > > +static inline void htonlsab(void *h, void *n, u16 len) { memcpy(n, h, > > > len); }; > > > +static inline void ntohlsab(void *n, void *h, u16 len) { memcpy(h, n, > > > len); }; > > > > > > memcpy is not defined to handle overlapping memory. Best to add: > > > if (n != h) > > > memcpy(...) > > > > Yes, but all usages of htonlsab()/ntohlsab() are non-overlapping. > > (overlapping/on place usage was replaced with htonlsab1()/ntohlsab1()). > > I see, but it would be safer to have that check in case you > used them in the wrong way?
I think this is no issue. Even if someone used them in a wrong way (which is improbable, as these functions are used just in a few places), memcpy() would probably do nothing when src==dst (although it is unspecified). > > > Might as well do that for these too: > > > +static inline void htonlsah(struct ospf_lsa_header *h, struct > > ospf_lsa_header *n) { *n = *h; }; > > > +static inline void ntohlsah(struct ospf_lsa_header *n, struct > > ospf_lsa_header *h) { *h = *n; }; > > > > *n = *h should be OK even if n == h, AFAIK. > > Hopyfully, but isn't gcc allowed to use memcpy to do that > assignment? Definitely isn't allowed to do something that breaks the specified behavior of C operators. > If you add that if (h!=n) test, doesn't the need for Xlasb1() go away as well? I think it is cleaner to have Xlsab1() for that purpose. -- Elen sila lumenn' omentielvo Ondrej 'SanTiago' Zajicek (email: santi...@crfreenet.org) OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) "To err is human -- to blame it on a computer is even more so."
signature.asc
Description: Digital signature