Fix for hal_endian.h SWAP16() macro

Fix for hal_endian.h SWAP16() macro

[Grant Edwards]

[-- Attachment #1: Type: text/plain, Size: 310 bytes --]


The SWAP16() macro is broken.

  unsigned u = SWAP16(0x1234);

u will be 0x123412 instead of 0x3412.

Here's a fix.

I believe that the SWAP32() macro will also break in a similar
way on systems where "int" is larger than 32 bits, but I have
no way to test that conjecture.

-- 
Grant Edwards
grante@visi.com

[-- Attachment #2: hal_endian.h--SWAP16.patch --]
[-- Type: text/plain, Size: 1156 bytes --]

? hal_endian.h--SWAP16.patch
Index: hal_endian.h
===================================================================
RCS file: /cvs/ecos/ecos/packages/hal/common/current/include/hal_endian.h,v
retrieving revision 1.3
diff -U10 -r1.3 hal_endian.h
--- hal_endian.h	23 May 2002 23:02:47 -0000	1.3
+++ hal_endian.h	24 Sep 2007 16:48:40 -0000
@@ -56,21 +56,21 @@
 #include <pkgconf/hal.h>                // CYGBLD_HAL_ENDIAN_H
 #include <cyg/infra/cyg_type.h>         // endian setting
 
 // Allow HALs to override the generic implementation of swap macros
 #ifdef CYGBLD_HAL_ENDIAN_H
 # include CYGBLD_HAL_ENDIAN_H
 #endif
 
 #ifndef CYG_SWAP16
 # define CYG_SWAP16(_x_)                                        \
-    ({ cyg_uint16 _x = (_x_); ((_x << 8) | (_x >> 8)); })
+    ({ cyg_uint16 _x = (_x_); (cyg_uint16)((_x << 8) | (_x >> 8)); })
 #endif
 
 #ifndef CYG_SWAP32
 # define CYG_SWAP32(_x_)                        \
     ({ cyg_uint32 _x = (_x_);                   \
        ((_x << 24) |                            \
        ((0x0000FF00UL & _x) <<  8) |            \
        ((0x00FF0000UL & _x) >>  8) |            \
        (_x  >> 24)); })
 #endif

Re: Fix for hal_endian.h SWAP16() macro

[Grant Edwards]

[-- Attachment #1: Type: text/plain, Size: 533 bytes --]

On Mon, Sep 24, 2007 at 11:57:01AM -0500, Grant Edwards wrote:

> The SWAP16() macro is broken.
> 
>   unsigned u = SWAP16(0x1234);
> 
> u will be 0x123412 instead of 0x3412.
> 
> Here's a fix.
> 
> I believe that the SWAP32() macro will also break in a similar
> way on systems where "int" is larger than 32 bits, but I have
> no way to test that conjecture.

OK, this time with a Changelog entry.  I have a hard time
remembering to edit the changelog since I use CVS to keep track
of such thing.

-- 
Grant Edwards
grante@visi.com

[-- Attachment #2: hal_endian.h--SWAP16.patch --]
[-- Type: text/plain, Size: 1762 bytes --]

Index: ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/hal/common/current/ChangeLog,v
retrieving revision 1.113
diff -U8 -r1.113 ChangeLog
--- ChangeLog	9 May 2006 15:51:39 -0000	1.113
+++ ChangeLog	27 Sep 2007 15:05:14 -0000
@@ -1,8 +1,13 @@
+2007-09-27  Grant Edwards <grante@visi.com>
+
+        * include/hal_endian.h (SWAP16): Fix "return value" so
+        that it only returns data in lower 16 bits instead of 24.
+
 2006-05-09  Andrew Lunn  <andrew.lunn@ascom.ch>
 
 	* src/hal_if.c (cyg_hal_diag_mangler_gdb_flush): Fix compiler
 	warning about signed/unsigned.
 
 2006-04-19  Alexander Neundorf <alexander.neundorf@jenoptik.com
 
 	* include/hal_if.h, src/hal_if.c: add a VV call for modifying
Index: include/hal_endian.h
===================================================================
RCS file: /cvs/ecos/ecos/packages/hal/common/current/include/hal_endian.h,v
retrieving revision 1.3
diff -U8 -r1.3 hal_endian.h
--- include/hal_endian.h	23 May 2002 23:02:47 -0000	1.3
+++ include/hal_endian.h	27 Sep 2007 15:05:14 -0000
@@ -58,17 +58,17 @@
 
 // Allow HALs to override the generic implementation of swap macros
 #ifdef CYGBLD_HAL_ENDIAN_H
 # include CYGBLD_HAL_ENDIAN_H
 #endif
 
 #ifndef CYG_SWAP16
 # define CYG_SWAP16(_x_)                                        \
-    ({ cyg_uint16 _x = (_x_); ((_x << 8) | (_x >> 8)); })
+    ({ cyg_uint16 _x = (_x_); (cyg_uint16)((_x << 8) | (_x >> 8)); })
 #endif
 
 #ifndef CYG_SWAP32
 # define CYG_SWAP32(_x_)                        \
     ({ cyg_uint32 _x = (_x_);                   \
        ((_x << 24) |                            \
        ((0x0000FF00UL & _x) <<  8) |            \
        ((0x00FF0000UL & _x) >>  8) |            \

Re: Fix for hal_endian.h SWAP16() macro

[Jonathan Larmour]

Grant Edwards wrote:
> On Mon, Sep 24, 2007 at 11:57:01AM -0500, Grant Edwards wrote:
> 
>> The SWAP16() macro is broken.
>>
>>   unsigned u = SWAP16(0x1234);
>>
>> u will be 0x123412 instead of 0x3412.
>>
>> Here's a fix.
>>
>> I believe that the SWAP32() macro will also break in a similar
>> way on systems where "int" is larger than 32 bits, but I have
>> no way to test that conjecture.

Feel free to make a guess.

> OK, this time with a Changelog entry.  I have a hard time
> remembering to edit the changelog since I use CVS to keep track
> of such thing.

Thanks, applied.

Jifl
-- 
eCosCentric Limited      http://www.eCosCentric.com/     The eCos experts
Barnwell House, Barnwell Drive, Cambridge, UK.       Tel: +44 1223 245571
Registered in England and Wales: Reg No 4422071.
  >>>> Visit us on stand 810 at The Embedded Systems Show 2007, NEC <<<<
  >>>> Oct 17-18 Birmingham, UK http://www.edaexhibitions.com/ess/  <<<<
------["The best things in life aren't things."]------      Opinions==mine