Hi Jukka,
2014-07-10 13:41 GMT+02:00 Jukka Rissanen <[email protected]>: > Hi Mario, > > On to, 2014-07-10 at 12:30 +0200, Mario Schuknecht wrote: > > Hi Jukka, > > > > > > 2014-07-10 10:12 GMT+02:00 Jukka Rissanen > > <[email protected]>: > > Hi Mario, > > > > On ke, 2014-07-09 at 16:52 +0200, Mario Schuknecht wrote: > > > 4-byte memory access works only when pointer is 4-byte > > aligned. Use memcpy > > > function, since 4-byte pointer alignment is not guaranteed. > > The same applies > > > to 2-byte memory access. > > > > > > What hardware do you see this issue? At least the Intel and > > ARM devices > > I have used work just fine as is. > > > > > > > > I see the problem on Marvell Kirkwood 88F6282 ARM SOC. The issue is > > reproducible. > > The pointer 'c' points to a 2-byte boundary. As a result the domain > > question Class > > field, that is before TTL field is set to zero and thus the cache > > contents is invalid. > > Ok, good to know. > > > > > > > I would prefer the use << operator instead of memcpy here. See > > usage > > examples in dnsproxy.c for details. > > > > > > Should I change it like this? > > /* now the 4 byte TTL field */ > > i = htonl(new_ttl); > > c[0] = (unsigned char)((i) & 0xFF); > > > > c[1] = (unsigned char)((i>>8) & 0xFF); > > c[2] = (unsigned char)((i>>16) & 0xFF); > > c[3] = (unsigned char)((i>>24) & 0xFF); > > What about just doing > > c[0] = new_ttl >> 24 & 0xff; > c[1] = new_ttl >> 16 & 0xff; > c[2] = new_ttl >> 8 & 0xff; > c[3] = new_ttl & 0xff; > > That should give the same result and avoids htonl() call, also () are > not needed as >> is done before AND. > > > c += 4; > > len -= 4; > > if (len < 0) > > break; > > > > > > /* now the 2 byte rdlen field */ > > w = c[0] | c[1] << 8; > > > > c += ntohs(w) + 2; > > len -= ntohs(w) + 2; > > We can avoid ntohs() by this > > w = c[0] << 8 | c[1]; > c += w + 2; > len -= w + 2; > > and so on. > > > > > > > And > > > > > > /* now the query, which is a name and 2 16 bit words */ > > l = dns_name_length(c) + 1; > > c += l; > > w = c[0] | c[1] << 8; > > > > type = ntohs(w); > > Could be written as > > type = c[0] << 8 | c[1]; > > > > Cheers, > Jukka > > > > Thank you for the comment. I test the changes and send a new patch. Mario _______________________________________________ connman mailing list [email protected] https://lists.connman.net/mailman/listinfo/connman
