[U-Boot] net/dns.c: Fix broken endian handling in dns command

From: Bernhard Kaindl <bernhard.kaindl@thalesgroup.com>

The U-Boot dns command only worked in little-endian CPUs so far because it
was based on an antique version of the TADNS source which was using a broken
macro to read the shorts found in DNS reply messages by shifting the LSB from
the message into the CPU's MSB of a short int and the MSB from the stream into
the LSB part of the CPU's short int. So far, so twisted.

To correct the twisted bytes, the code used ntohs() as a byte-swapping function
to swap the MSB back where it belongs and vice versa.

This works fine, except that ntohs() naturally does nothing on big-endian CPUs.

So on big-endian CPUs, the MSB from the message stayed in the LSB of the CPU
and vice versa.

Ditch this brain-deadness by just shifting the MSB from the network byte
stream of the reply message into the right (MSB) location of a short int
and putting the LSB from the network byte stream as the lower byte of it,
and we are done with reading the short from the network stream for both
endianesses, no ntohs() or such!

The current TADNS source uses the same macro meanwhile.

Signed-off-by: Bernhard Kaindl <bernhard.kaindl@thalesgroup.com>
Cc: Pieter Voorthuijsen <pieter.voorthuijsen@prodrive.nl>
Cc: Robin Getz <rgetz@blackfin.uclinux.org>
---
 net/dns.c |   22 ++++++++++------------
 1 files changed, 10 insertions(+), 12 deletions(-)

On Wednesday 12 October 2011 17:23:20 Bernhard Kaindl wrote:
> The U-Boot dns command only worked in little-endian CPUs so far because it
> was based on an antique version of the TADNS source which was using a
> broken macro to read the shorts found in DNS reply messages by shifting
> the LSB from the message into the CPU's MSB of a short int and the MSB
> from the stream into the LSB part of the CPU's short int. So far, so
> twisted.
> 
> To correct the twisted bytes, the code used ntohs() as a byte-swapping
> function to swap the MSB back where it belongs and vice versa.
> 
> This works fine, except that ntohs() naturally does nothing on big-endian
> CPUs.
> 
> So on big-endian CPUs, the MSB from the message stayed in the LSB of the
> CPU and vice versa.
> 
> Ditch this brain-deadness by just shifting the MSB from the network byte
> stream of the reply message into the right (MSB) location of a short int
> and putting the LSB from the network byte stream as the lower byte of it,
> and we are done with reading the short from the network stream for both
> endianesses, no ntohs() or such!

please use a standard macro instead of inventing yet another.  we've got the 
rich Linux api which should cover every case you could possibly need.
	cpu_to_{l,b}e{16,32,64}(...)
	{l,b}e_to_cpu{16,32,63}(...)

see include/linux/byteorder/
-mike

Am 12.10.2011 23:48, schrieb Mike Frysinger:
>>
>> Ditch this brain-deadness by just shifting the MSB from the network byte
>> stream of the reply message into the right (MSB) location of a short int
>> and putting the LSB from the network byte stream as the lower byte of it,
>> and we are done with reading the short from the network stream for both
>> endianesses, no ntohs() or such!
>
> please use a standard macro instead of inventing yet another.  we've got the
> rich Linux api which should cover every case you could possibly need.
> 	cpu_to_{l,b}e{16,32,64}(...)
> 	{l,b}e_to_cpu{16,32,63}(...)
>
> see include/linux/byteorder/
> -mike

No, of course I looked, but didn't find what is needed.

These are just conditional byte swaps in 16,32 or 64 bit, not more.

The code needs is a conversion from an uneven "const char *" to u16!

It ___is___ possible to use be_to_cpu16, but it needs ugly casting:

         if (&p[5] > e || be_to_cpu16(*(u16 *)(p+1)) != DNS_A_RECORD) {

To not be distracted by reading the ugly casting, one would have to put 
this into a macro again, but if you really like to see be_to_cpu16 so 
much, I can change the macro to:

#define netstring_to_cpu_short(p)    be16_to_cpu(*(u16 *)(p))

This result doesn't make it more readable than what I already posted.

I'd prefer the already sent patch.

Best Regards,
Bernhard

On Friday 14 October 2011 03:44:13 Bernhard Kaindl wrote:
> Am 12.10.2011 23:48, schrieb Mike Frysinger:
> >> Ditch this brain-deadness by just shifting the MSB from the network byte
> >> stream of the reply message into the right (MSB) location of a short int
> >> and putting the LSB from the network byte stream as the lower byte of
> >> it, and we are done with reading the short from the network stream for
> >> both endianesses, no ntohs() or such!
> > 
> > please use a standard macro instead of inventing yet another.  we've got
> > the rich Linux api which should cover every case you could possibly
> > need.
> > 
> > 	cpu_to_{l,b}e{16,32,64}(...)
> > 	{l,b}e_to_cpu{16,32,63}(...)
> > 
> > see include/linux/byteorder/
> 
> No, of course I looked, but didn't find what is needed.
> 
> These are just conditional byte swaps in 16,32 or 64 bit, not more.
> 
> The code needs is a conversion from an uneven "const char *" to u16!
> 
> It ___is___ possible to use be_to_cpu16, but it needs ugly casting:
> 
>          if (&p[5] > e || be_to_cpu16(*(u16 *)(p+1)) != DNS_A_RECORD) {
> 
> To not be distracted by reading the ugly casting, one would have to put
> this into a macro again, but if you really like to see be_to_cpu16 so
> much, I can change the macro to:
> 
> #define netstring_to_cpu_short(p)    be16_to_cpu(*(u16 *)(p))
> 
> This result doesn't make it more readable than what I already posted.

sounds like you want get_unaligned_be16()
-mike