Re: [ECOS] Re: CGY_SWAP16() seems broken to me.

[Sergei Gavrikov <w3sg@SoftHome.net>]

On Sat, Sep 22, 2007 at 03:20:51PM +0000, Grant Edwards wrote:
> On 2007-09-22, Grant Edwards <grante@visi.com> wrote:
> 
> > My fix above should be completely transparent. if the value is
> > being assigned to a 16 bit lvalue, the compiler will generate
> > exactly the same code as when using the existing "broken"
> > macro. If the result from CYG_SWAP16() is being masked by the
> > user, the fixed macro will generate exactly the same code as
> > the existing macro.
> >
> > The only argument against my fix would be to claim that
> > CYG_SWAP16 is expected to return 24 bits in wider contexts, and
> > returning only 16 bits is going to break code or surprise a
> > user.
> 
> I've examined all uses of SWAP16 and the LE16/BE16 macros in my
> source tree (which is a few months out of date), and it doesn't
> look like any existing code depends on the fact that SWAP16()
> produces 24 bits instead of 16.

Today's score for CYG_SWAP16() is

CYG_SWAP16()      : 3 sources
CYG_CPU_TO_LE16() : 4 sources 

As you could see, ntohs(), htons() = swap16() in the little-endian world
(net/tcpip/current/include/sys/endian.h). There are tons of C lines are
using these net macros in the net stuff and at first, I just did think
about same strong rules for the HAL CYG_SWAP16() too, i.e. about type
casting (compiler care) + bit masking (CPU workaround).

I do see no criminal if you will provide a patch, which will set the
first barier - type casting. I vote for your patch. I did dislike that
0x123412.


	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