This is the mail archive of the
ecos-discuss@sourceware.org
mailing list for the eCos project.
Re: CGY_SWAP16() seems broken to me.
- From: Sergei Gavrikov <w3sg at SoftHome dot net>
- To: Grant Edwards <grante at visi dot com>
- Cc: ecos-discuss at sources dot redhat dot com
- Date: Sat, 22 Sep 2007 15:20:26 +0300
- Subject: Re: [ECOS] CGY_SWAP16() seems broken to me.
- References: <fd2091$bnn$1@sea.gmane.org>
On Sat, Sep 22, 2007 at 02:56:33AM +0000, Grant Edwards wrote:
> unsigned u = CYG_SWAP16(0x1234);
>
> What would a reasonable person (who hasn't looked at
> hal_endian.h) expect u to be?
>
> I certainly expected it to be 0x3412.
>
> But it's not. It's 0x123412.
That is exists. I got same result
#include <cyg/hal/hal_endian.h>
#include <sys/param.h>
#include <sys/endian.h>
#include <stdio.h>
int
main (void)
{
printf ("HAL swap16(0x1234) = 0x%x\n", CYG_SWAP16 (0x1234));
printf ("NET swap16(0x1234) = 0x%x\n", swap16 (0x1234));
return 0;
}
HAL swap16(0x1234) = 0x123412
NET swap16(0x1234) = 0x3412
> IMO, that's just plain broken.
... I don't know. What do eCos experts think about?
> Here's the definition from hal_endian.h:
>
> # define CYG_SWAP16(_x_) \
> ({ cyg_uint16 _x = (_x_); ((_x << 8) | (_x >> 8)); })
>
> Why isn't it this?
>
> # define CYG_SWAP16(_x_) \
> ({ cyg_uint16 _x = (_x_); (cyg_uint16)((_x << 8) | (_x >> 8)); })
>
> Why would anybody expect a macro named CYG_SWAP16() to return a
> "24-bit value" when passed a "16-bit value"?
Usually, they use yet masking in same macros. Good byteorder examples
are
net/bsd_tcpip/current/include/sys/endian.h
net/tcpip/current/include/sys/endian.h
It seems, that eCos HAL's CYG_SWAP16() either assumes what it will be to
live in "16-bit world" or it assumes that will be a developers's care. I
did find only 3 places where CYG_SWAP16() macro is used. For example,
CYG_SWAP16() macro uses itself in the eCos drivers for 16-bit Ethernet
controllers (cs8900a, ec555) "16-bit world". And I found that RedBoot's
conf_endian_fixup() (fconfig.c) uses eCos HAL's CYG_SWAP16() "as is",
because that was a "developer's care", there is there
u16 = CYG_SWAP16(u16);
but, not :-)
unsigned u = CYG_SWAP16(0x1234);
In any case, thank you for your report, we will be to know about.
Sergei
--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss