Ethernet Bridge development
 help / color / mirror / Atom feed
* [Bridge] Update to RSTP lib
@ 2009-01-12 19:22 Manish Talreja
  2009-01-12 20:00 ` richardvoigt
  0 siblings, 1 reply; 6+ messages in thread
From: Manish Talreja @ 2009-01-12 19:22 UTC (permalink / raw)
  To: bridge@lists.linux-foundation.org


[-- Attachment #1.1: Type: text/plain, Size: 382 bytes --]

We have been using the RSTP implementation posted at this location:
https://lists.linux-foundation.org/pipermail/bridge/2008-May/005859.html

We have modified rstp.c and rstp.h to work with compilers other than gcc. Attached are two patch files that show our changes. Currently, we have tested this code with the Diab compiler (v 4.4a) and with IAR compiler (v 5.11).

Manish

[-- Attachment #1.2: Type: text/html, Size: 2281 bytes --]

[-- Attachment #2: rstp.c.patch --]
[-- Type: application/octet-stream, Size: 13990 bytes --]

44a46,53
> #ifndef __GNUC__
> #define CMP(_a, _op, _b)                                           \
> (memcmp(&(_a), &(_b),                                              \
>   __builtin_choose_expr(sizeof(_a) == sizeof(_b),                  \
>                         sizeof(_a),                                \
>                         0)                                   \
> ) _op 0)
> #else
50a60
> #endif
54a65,72
> #ifndef __GNUC__
> #define CPY(_a, _b)                                           \
> (memcpy(&(_a), &(_b),                                              \
>   __builtin_choose_expr(sizeof(_a) == sizeof(_b),                  \
>                         sizeof(_a),                                \
>                         0)                                   \
> ))
> #else
60a79,80
> #endif
> 
73a94,100
> #ifndef __GNUC__
> struct _dummy_user_ref_struct
> {
>   void *user_ref;
> } _user_ref_var = { NULL },
>   *BRIDGE = &_user_ref_var, *PORT = &_user_ref_var;
> #else
78a106
> #endif
80a109,116
> #ifndef __GNUC__
> #define DEBUG(...) \
>   STP_OUT_logmsg(BRIDGE->user_ref, PORT->user_ref, STP_LOG_LEVEL_DEBUG, __VA_ARGS__)
> #define ERROR(...) \
>   STP_OUT_logmsg(BRIDGE->user_ref, PORT->user_ref, STP_LOG_LEVEL_ERROR, __VA_ARGS__)
> #define ABORT(...) \
>   { ERROR(__VA_ARGS__);}
> #else
86a123
> #endif
104a142,144
> #ifndef __GNUC__
> #define STATES(...) enum _CAT(STM_NAME, states) { STN(BEGIN), __VA_ARGS__ }
> #else
105a146
> #endif
107a149,158
> #ifndef __GNUC__
> #define TR(_cond, _new_state) \
> {                                                                           \
>   if (_cond) {                                                               \
>     new_state = STN(_new_state);                                             \
>     DEBUG("%s: Transition to state %s\n", _STR(STM_NAME), #_new_state);      \
>     goto STM_LABEL(_new_state);                                              \
>   }                                                                          \
> }
> #else
115a167
> #endif
122a175,180
> #ifndef __GNUC__
> #define GC(...)      \
>          global_conditions:      \
>              __VA_ARGS__         \
>              return new_state
> #else
126a185
> #endif
128a188,194
> #ifndef __GNUC__
> #define BG(...)                                              \
>          case STN(BEGIN):                                                \
>              __VA_ARGS__                                                \
>              ABORT("%s: No where to go from BEGIN\n", _STR(STM_NAME));   \
>              return -1
> #else
133a200
> #endif
144a212,215
> #ifndef __GNUC__
> #define STM_FUNC_DECL(_type, ...) \
>   int _type(int state, int single_step, __VA_ARGS__)
> #else
146a218,219
> #endif
> 
150a224,236
> #ifndef __GNUC__
> #define STM(_args, ...)                                 \
> static STM_FUNC_DECL(STM_FUNC(STM_NAME), _args)                  \
> {                                                                \
>   int new_state = 0;                                             \
>   switch (state) {                                               \
>     __VA_ARGS__                                                  \
>       default:                                                   \
>     ABORT("%s: Got unknown state %d\n", __func__, state);        \
>     return -1;                                                   \
>   }                                                              \
> }
> #else
161a248,249
> #endif
> 
164a253,262
> #ifndef __GNUC__
> #define STM_RUN(_state, _transitioned, _func, ...) \
> {                                                               \
>   int new_state = _func(*_state, 0/*no single step*/, __VA_ARGS__);    \
>   if (new_state > 0) {                                           \
>     *_state = new_state;                                         \
>     *_transitioned = TRUE;                                       \
>   }                                                              \
> }
> #else
172a271
> #endif
173a273,278
> #ifndef __GNUC__
> #define STM_BEGIN(_state, _func, ...) \
> {                                                               \
>   *_state = _func(0/*BEGIN*/, 1/*single step*/, __VA_ARGS__);          \
> }
> #else
177a283,284
> #endif
> 
191c298,301
< typedef struct _BridgeId {
---
> #ifdef __ICCARM__
> #pragma pack(1)
> #endif
> typedef COMPILER_INLINE_PACK struct _BridgeId {
195,196c305,312
< 
< typedef struct {
---
> #ifdef __ICCARM__
> #pragma pack()
> #endif
> 
> #ifdef __ICCARM__
> #pragma pack(1)
> #endif
> typedef COMPILER_INLINE_PACK struct _PathCost{
198a315,317
> #ifdef __ICCARM__
> #pragma pack()
> #endif
206a326,337
> #ifndef __GNUC__
> static inline PathCost path_cost_add(PathCost x, uint y)
> {
>   PathCost r;
> 
>   uint z = y +
>     (x.cost[0] << 24) + (x.cost[1] << 16) + (x.cost[2] << 8) + (x.cost[3]);
> 
>   r.cost[0] = (z >> 24);
>   r.cost[1] = (z >> 16) & 0xff;
>   r.cost[2] = (z >> 8) & 0xff;
>   r.cost[3] = z & 0xff;
207a339,341
>   return r;
> }
> #else
222c356,361
< typedef struct _PortId {
---
> #endif
> 
> #ifdef __ICCARM__
> #pragma pack(1)
> #endif
> typedef COMPILER_INLINE_PACK struct _PortId {
224a364,366
> #ifdef __ICCARM__
> #pragma pack()
> #endif
231c373,376
< typedef struct _PriorityVector
---
> #ifdef __ICCARM__
> #pragma pack(1)
> #endif
> typedef COMPILER_INLINE_PACK struct _PriorityVector
239c384,390
< 
---
> #ifdef __ICCARM__
> #pragma pack()
> #endif
> 
> #ifdef __ICCARM__
> #pragma pack(1)
> #endif
241c392
< typedef struct _PriorityVector4
---
> typedef COMPILER_INLINE_PACK struct _PriorityVector4
248c399,401
< 
---
> #ifdef __ICCARM__
> #pragma pack()
> #endif
317c470,473
< typedef struct _RawBPDU
---
> #ifdef __ICCARM__
> #pragma pack(1)
> #endif
> typedef COMPILER_INLINE_PACK struct _RawBPDU
321a478
> #ifdef __GNUC__
323a481
> #endif
341a500
> #ifdef __GNUC__
343c502
< 
---
> #endif
345a505
> #ifdef __GNUC__
347c507
< 
---
> #endif
348a509,511
> #ifdef __ICCARM__
> #pragma pack()
> #endif
350a514,519
> #ifndef __GNU_C
> // Markers not allowed
> #define TCN_END_SIZE_BYTES    4
> #define CONFIG_END_SIZE_BYTES (sizeof(RawBPDU) - sizeof(uchar))
> #define RST_END_SIZE_BYTES    (sizeof(RawBPDU))
> #endif
827a997,1010
> #ifndef __GNUC__
> static bool g_ForAllPortBool;
> #define ForAllPort(result, expr)                                                     \
> {                                                                           \
>   Port *FPORT;                                                               \
>   bool res = TRUE;                                                           \
>   for (FPORT = BRIDGE->port_list; FPORT != NULL; FPORT = FPORT->port_next)   \
>     if (!(expr)) {                                                           \
>       res = FALSE;                                                           \
>       break;                                                                 \
>     }                                                                        \
>   result = res;                                                                       \
> }
> #else
838a1022
> #endif
843a1028,1035
> #ifndef __GNUC__
> #define ForAllPortDo(...)                                          \
> {                                                                           \
>   Port *FPORT;                                                               \
>   for (FPORT = BRIDGE->port_list; FPORT != NULL; FPORT = FPORT->port_next)   \
>     { __VA_ARGS__ }                                                           \
> }
> #else
849a1042
> #endif
860c1053
< #define allSynced ForAllPort(FPORT->_synced || FPORT->_role == RootPort)
---
> #define allSynced(result) ForAllPort(result, FPORT->_synced || FPORT->_role == RootPort)
881c1074
< #define reRooted ForAllPort(FPORT == PORT || FPORT->_rrWhile == 0)
---
> #define reRooted(result) ForAllPort(result, FPORT == PORT || FPORT->_rrWhile == 0)
970a1164,1168
> #ifndef __GNUC__
> #define set_tcWhile(_val) \
> { uint _oldval = tcWhile; tcWhile = _val; \
>    update_tcWhile_count(BRIDGE_ARGS, (tcWhile?1:0) - (_oldval?1:0)); }
> #else
974c1172
< 
---
> #endif
1116c1314,1316
<   if (ForAllPort(FPORT->_reselect == FALSE))
---
>   bool temp;
>   ForAllPort(temp, FPORT->_reselect == FALSE);
>   if (temp)
1169a1370
>   int flags;
1177c1378
<   int flags = 0;
---
>   flags = 0;
1184a1386,1388
> #ifndef __GNUC__
>   STP_OUT_tx_bpdu(PORT->user_ref, &b, CONFIG_END_SIZE_BYTES);
> #else
1185a1390
> #endif
1191a1397,1398
>   int flags;
>   int r;
1199c1406
<   int flags = 0;
---
>   flags = 0;
1201c1408
<   int r = role;
---
>   r = role;
1220a1428,1430
> #ifndef __GNUC__
>   STP_OUT_tx_bpdu(PORT->user_ref, &b, RST_END_SIZE_BYTES);
> #else
1221a1432
> #endif
1233a1445,1447
> #ifndef __GNUC__
>   STP_OUT_tx_bpdu(PORT->user_ref, &b, TCN_END_SIZE_BYTES);
> #else
1234a1449,1450
> #endif
> 
1422c1638
<     GC();
---
>     GC(;);
1505c1721
<     GC();
---
>     GC(;);
1549c1765
<     GC();
---
>     GC(;);
1589c1805
<     GC();
---
>     GC(;);
1775c1991
<     GC();
---
>     GC(;);
1792c2008,2009
<        TR(!ForAllPort(!FPORT->_reselect), ROLE_SELECTION);
---
>        ForAllPort(g_ForAllPortBool,!FPORT->_reselect);
>        TR(!g_ForAllPortBool, ROLE_SELECTION);
1938c2155,2156
<        TR(((allSynced && !agree) || (proposed && agree)) && xc, ROOT_AGREED);
---
>        allSynced(g_ForAllPortBool);
>        TR(((g_ForAllPortBool && !agree) || (proposed && agree)) && xc, ROOT_AGREED);
1941a2160
>        reRooted(g_ForAllPortBool);
1943c2162
<            ((reRooted && (rbWhile == 0)) && (rstpVersion))) && !learn && xc,
---
>            ((g_ForAllPortBool && (rbWhile == 0)) && (rstpVersion))) && !learn && xc,
1945c2164,2165
<        TR((fdWhile == 0 || ((reRooted && (rbWhile == 0)) && (rstpVersion)))
---
>        reRooted(g_ForAllPortBool);
>        TR((fdWhile == 0 || ((g_ForAllPortBool && (rbWhile == 0)) && (rstpVersion)))
2061c2281,2282
<        TR(((allSynced && !agree) || (proposed && agree)) && xc,
---
>        allSynced(g_ForAllPortBool);
>        TR(((g_ForAllPortBool && !agree) || (proposed && agree)) && xc,
2083c2304
<     GC();
---
>     GC(;);
2128c2349
<     GC();
---
>     GC(;);
2250a2472,2474
>   int i;
>   bool transitioned;
> 
2254,2255d2477
<   int i;
<   bool transitioned;
2270a2493
>   int i;
2274d2496
<   int i;
2522a2745,2747
>   Times new_times;
>   bool set_times;
> 
2543,2544c2768,2769
<   Times new_times = BridgeTimes;
<   bool set_times = FALSE;
---
>   new_times = BridgeTimes;
>   set_times = FALSE;
2784a3010,3019
> #ifndef __GNUC__
> #define SET_CFG(_type, _arg, _field, _value, _retval) \
> {                                                           \
>   STP_ ## _type ## Config cfg;                               \
>   ZERO(cfg);                                                 \
>   cfg._type ## _ ## _field = _value;                         \
>   cfg. set_ ## _type ## _ ## _field = 1;                     \
>   _retval = STP_IN_set_ ## _type ## _config(_arg, &cfg);               \
> }
> #else
2792a3028
> #endif
2798c3034,3036
<   return SET_CFG(bridge, BRIDGE, protocol_version, version);
---
>   bool retval;
>   SET_CFG(bridge, BRIDGE, protocol_version, version, retval);
>   return retval;
2803c3041,3043
<   return SET_CFG(bridge, BRIDGE, address, *addr);
---
>   bool retval;
>   SET_CFG(bridge, BRIDGE, address, *addr, retval);
>   return retval;
2808c3048,3050
<   return SET_CFG(bridge, BRIDGE, priority, priority);
---
>   bool retval;
>   SET_CFG(bridge, BRIDGE, priority, priority, retval);
>   return retval;
2813c3055,3057
<   return SET_CFG(bridge, BRIDGE, hello_time, hello_time);
---
>   bool retval;
>   SET_CFG(bridge, BRIDGE, hello_time, hello_time, retval);
>   return retval;
2818c3062,3064
<   return SET_CFG(bridge, BRIDGE, max_age, max_age);
---
>   bool retval;
>   SET_CFG(bridge, BRIDGE, max_age, max_age, retval);
>   return retval;
2824c3070,3072
<   return SET_CFG(bridge, BRIDGE, forward_delay, fwd_delay);
---
>   bool retval;
>   SET_CFG(bridge, BRIDGE, forward_delay, fwd_delay, retval);
>   return retval;
2830c3078,3080
<   return SET_CFG(bridge, BRIDGE, tx_hold_count, count);
---
>   bool retval;
>   SET_CFG(bridge, BRIDGE, tx_hold_count, count, retval);
>   return retval;
2838c3088,3090
<   return SET_CFG(port, PORT, priority, priority);
---
>   bool retval;
>   SET_CFG(port, PORT, priority, priority, retval);
>   return retval;
2843c3095,3097
<   return SET_CFG(port, PORT, pathcost, pcost);
---
>   bool retval;
>   SET_CFG(port, PORT, pathcost, pcost, retval);
>   return retval;
2849c3103,3105
<   return SET_CFG(port, PORT, admin_edge, edge);
---
>   bool retval;
>   SET_CFG(port, PORT, admin_edge, edge, retval);
>   return retval;
2854c3110,3112
<   return SET_CFG(port, PORT, auto_edge, edge);
---
>   bool retval;
>   SET_CFG(port, PORT, auto_edge, edge, retval);
>   return retval;
2859c3117,3119
<   return SET_CFG(port, PORT, admin_p2p, p2p);
---
>   bool retval;
>   SET_CFG(port, PORT, admin_p2p, p2p, retval);
>   return retval;
2959a3220
>   Bridge *BRIDGE;
2969c3230
<   Bridge *BRIDGE = b;
---
>   BRIDGE = b;
3005a3267,3269
>   Port *p;
>   Port *PORT;
> 
3011c3275
<   Port *p = STP_OUT_mem_zalloc(sizeof(Port));
---
>   p = STP_OUT_mem_zalloc(sizeof(Port));
3024c3288
<   Port *PORT = p;
---
>   PORT = p;
3050a3315
>   Port **prev; 
3059c3324
<   Port **prev = &BRIDGE->port_list;
---
>   prev = &BRIDGE->port_list;

[-- Attachment #3: rstp.h.patch --]
[-- Type: application/octet-stream, Size: 482 bytes --]

225a226,250
> 
> 
> 
> #ifdef __GNUC__
> #define COMPILER_INLINE_PACK
> #elif defined(__DCC__)
> #define COMPILER_INLINE_PACK  __packed__
> #elif defined (__ICCARM__)
> #define COMPILER_INLINE_PACK
> #else
> DEFINE PACKING FOR YOUR COMPILER
> #endif
> 
> #ifndef __GNUC__
> #define __attribute__(x)
> #define __builtin_choose_expr(a,b,c) ((a) ? (b) : (c))
> #endif
> 
> #ifdef __DCC__
> #define inline __inline__
> #define __func__  "\0"
> #endif
> 
> 
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Bridge] Update to RSTP lib
  2009-01-12 19:22 [Bridge] Update to RSTP lib Manish Talreja
@ 2009-01-12 20:00 ` richardvoigt
  2009-01-12 20:42   ` Manish Talreja
  0 siblings, 1 reply; 6+ messages in thread
From: richardvoigt @ 2009-01-12 20:00 UTC (permalink / raw)
  To: Manish Talreja; +Cc: bridge@lists.linux-foundation.org

If you want your patch to be understandable, create it as a contextual
diff with the -u option.

Do the pack pragmas really need to be conditional?  Pretty much every
compiler will do the right thing on encountering those.  (Using
__attribute__ as well, for compilers that support it, isn't a bad
thing but no need to avoid #pragma pack on gcc).

On Mon, Jan 12, 2009 at 1:22 PM, Manish Talreja <mtalreja@crestron.com> wrote:
> We have been using the RSTP implementation posted at this location:
>
> https://lists.linux-foundation.org/pipermail/bridge/2008-May/005859.html
>
>
>
> We have modified rstp.c and rstp.h to work with compilers other than gcc.
> Attached are two patch files that show our changes. Currently, we have
> tested this code with the Diab compiler (v 4.4a) and with IAR compiler (v
> 5.11).
>
>
>
> Manish
>
> _______________________________________________
> Bridge mailing list
> Bridge@lists.linux-foundation.org
> https://lists.linux-foundation.org/mailman/listinfo/bridge
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Bridge] Update to RSTP lib
  2009-01-12 20:00 ` richardvoigt
@ 2009-01-12 20:42   ` Manish Talreja
  2009-01-12 21:32     ` Stephen Hemminger
  0 siblings, 1 reply; 6+ messages in thread
From: Manish Talreja @ 2009-01-12 20:42 UTC (permalink / raw)
  To: bridge@lists.linux-foundation.org

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

Here are the unified diff files as requested.

You are right about #pragma pack(). That's supported on pretty much all compilers. However, __attribute__ is not supported on all compilers and that's what was used in the original code. Our goal was to leave the gcc syntax the way it was in the original code. If you are ok with getting rid of the __attribute__((packed)), the code will be a lot more readable.

-Manish


-----Original Message-----
From: richardvoigt@gmail.com [mailto:richardvoigt@gmail.com]
Sent: Monday, January 12, 2009 3:01 PM
To: Manish Talreja
Cc: bridge@lists.linux-foundation.org
Subject: Re: [Bridge] Update to RSTP lib

If you want your patch to be understandable, create it as a contextual
diff with the -u option.

Do the pack pragmas really need to be conditional?  Pretty much every
compiler will do the right thing on encountering those.  (Using
__attribute__ as well, for compilers that support it, isn't a bad
thing but no need to avoid #pragma pack on gcc).

On Mon, Jan 12, 2009 at 1:22 PM, Manish Talreja <mtalreja@crestron.com> wrote:
> We have been using the RSTP implementation posted at this location:
>
> https://lists.linux-foundation.org/pipermail/bridge/2008-May/005859.html
>
>
>
> We have modified rstp.c and rstp.h to work with compilers other than gcc.
> Attached are two patch files that show our changes. Currently, we have
> tested this code with the Diab compiler (v 4.4a) and with IAR compiler (v
> 5.11).
>
>
>
> Manish
>
> _______________________________________________
> Bridge mailing list
> Bridge@lists.linux-foundation.org
> https://lists.linux-foundation.org/mailman/listinfo/bridge
>

[-- Attachment #2: rstp.h.unified.patch --]
[-- Type: application/octet-stream, Size: 618 bytes --]

--- /code/originalrstp/rstp.h	Wed Feb 27 13:05:50 2008
+++ /code/RSTP/rstp.h	Wed Oct 29 12:27:56 2008
@@ -223,4 +223,29 @@
 
 void STP_OUT_mem_free(void *);
 
+
+
+
+#ifdef __GNUC__
+#define COMPILER_INLINE_PACK
+#elif defined(__DCC__)
+#define COMPILER_INLINE_PACK  __packed__
+#elif defined (__ICCARM__)
+#define COMPILER_INLINE_PACK
+#else
+DEFINE PACKING FOR YOUR COMPILER
+#endif
+
+#ifndef __GNUC__
+#define __attribute__(x)
+#define __builtin_choose_expr(a,b,c) ((a) ? (b) : (c))
+#endif
+
+#ifdef __DCC__
+#define inline __inline__
+#define __func__  "\0"
+#endif
+
+
+
 #endif

[-- Attachment #3: rstp.c.unified.patch --]
[-- Type: application/octet-stream, Size: 26659 bytes --]

--- /code/originalrstp/rstp.c	Tue Apr 01 05:48:28 2008
+++ /code/RSTP/rstp.c	Wed Oct 29 12:27:55 2008
@@ -42,22 +43,41 @@
 
 
 /* memcmp wrapper, validating size equality of _a and _b */
+#ifndef __GNUC__
+#define CMP(_a, _op, _b)                                           \
+(memcmp(&(_a), &(_b),                                              \
+  __builtin_choose_expr(sizeof(_a) == sizeof(_b),                  \
+                        sizeof(_a),                                \
+                        0)                                   \
+) _op 0)
+#else
 #define CMP(_a, _op, _b)                                           \
 (memcmp(&(_a), &(_b),                                              \
   __builtin_choose_expr(sizeof(_a) == sizeof(_b),                  \
                         sizeof(_a),                                \
                         (void)0)                                   \
 ) _op 0)
+#endif
 
 #define ZERO(_a) memset(&(_a), 0, sizeof(_a))
 
 /* memcmp wrapper, validating size equality of _a and _b */
+#ifndef __GNUC__
+#define CPY(_a, _b)                                           \
+(memcpy(&(_a), &(_b),                                              \
+  __builtin_choose_expr(sizeof(_a) == sizeof(_b),                  \
+                        sizeof(_a),                                \
+                        0)                                   \
+))
+#else
 #define CPY(_a, _b)                                           \
 (memcpy(&(_a), &(_b),                                              \
   __builtin_choose_expr(sizeof(_a) == sizeof(_b),                  \
                         sizeof(_a),                                \
                         (void)0)                                   \
 ))
+#endif
+
 
 #define BRIDGE_ARGS_PROTO        Bridge *BRIDGE
 #define PORT_ARGS_PROTO          Bridge *BRIDGE, Port *PORT
@@ -71,19 +91,36 @@
    That way log messages can use BRIDGE and PORT always.
 */
 
+#ifndef __GNUC__
+struct _dummy_user_ref_struct
+{
+  void *user_ref;
+} _user_ref_var = { NULL },
+  *BRIDGE = &_user_ref_var, *PORT = &_user_ref_var;
+#else
 struct _dummy_user_ref_struct
 {
   void *user_ref;
 } _user_ref_var = { .user_ref = NULL},
   *BRIDGE = &_user_ref_var, *PORT = &_user_ref_var;
+#endif
 
 /* Logging macros */
+#ifndef __GNUC__
+#define DEBUG(...) \
+  STP_OUT_logmsg(BRIDGE->user_ref, PORT->user_ref, STP_LOG_LEVEL_DEBUG, __VA_ARGS__)
+#define ERROR(...) \
+  STP_OUT_logmsg(BRIDGE->user_ref, PORT->user_ref, STP_LOG_LEVEL_ERROR, __VA_ARGS__)
+#define ABORT(...) \
+  { ERROR(__VA_ARGS__);}
+#else
 #define DEBUG(fmt...) \
   STP_OUT_logmsg(BRIDGE->user_ref, PORT->user_ref, STP_LOG_LEVEL_DEBUG, fmt)
 #define ERROR(fmt...) \
   STP_OUT_logmsg(BRIDGE->user_ref, PORT->user_ref, STP_LOG_LEVEL_ERROR, fmt)
 #define ABORT(fmt...) \
   ({ ERROR(fmt); *(int *)0 = 1; })
+#endif
 
 /***************** Macros for state machine descriptions ********************/
 
@@ -102,9 +139,23 @@
 #define STM_FUNC(_name) _CAT(_name, func)
 
 /* BEGIN is state 0 in all state machines */
+#ifndef __GNUC__
+#define STATES(...) enum _CAT(STM_NAME, states) { STN(BEGIN), __VA_ARGS__ }
+#else
 #define STATES(_args...) enum _CAT(STM_NAME, states) { STN(BEGIN), _args }
+#endif
 
 /* Transition definition */
+#ifndef __GNUC__
+#define TR(_cond, _new_state) \
+{                                                                           \
+  if (_cond) {                                                               \
+    new_state = STN(_new_state);                                             \
+    DEBUG("%s: Transition to state %s\n", _STR(STM_NAME), #_new_state);      \
+    goto STM_LABEL(_new_state);                                              \
+  }                                                                          \
+}
+#else
 #define TR(_cond, _new_state) \
 ({                                                                           \
   if (_cond) {                                                               \
@@ -113,6 +164,7 @@
     goto STM_LABEL(_new_state);                                              \
   }                                                                          \
 })
+#endif
 
 /* Global conditions: Transition conditions that don't depend on current state.
    In our state machine, they get checked after those that are specific to the
@@ -120,17 +172,32 @@
    new_state is the state we last transitioned to, or 0 if no transition
    was made.
 */
+#ifndef __GNUC__
+#define GC(...)      \
+         global_conditions:      \
+             __VA_ARGS__         \
+             return new_state
+#else
 #define GC(_transitions...)      \
          global_conditions:      \
              _transitions        \
              return new_state
+#endif
 
 /* Transitions from BEGIN state */
+#ifndef __GNUC__
+#define BG(...)                                              \
+         case STN(BEGIN):                                                \
+             __VA_ARGS__                                                \
+             ABORT("%s: No where to go from BEGIN\n", _STR(STM_NAME));   \
+             return -1
+#else
 #define BG(_transitions...)                                              \
          case STN(BEGIN):                                                \
              _transitions                                                \
              ABORT("%s: No where to go from BEGIN\n", _STR(STM_NAME));   \
              return -1
+#endif
 
 /* State definition */
 #define ST(_state, _actions, _transitions) \
@@ -142,12 +209,31 @@
              _transitions                  \
              goto global_conditions
 
+#ifndef __GNUC__
+#define STM_FUNC_DECL(_type, ...) \
+  int _type(int state, int single_step, __VA_ARGS__)
+#else
 #define STM_FUNC_DECL(_type, _args...) \
   int _type(int state, int single_step, _args)
+#endif
+
 
 /* This just barely works. _args expansion includes commas */
 //int STM_FUNC(STM_NAME)(int state, int single_step, _args)
 
+#ifndef __GNUC__
+#define STM(_args, ...)                                 \
+static STM_FUNC_DECL(STM_FUNC(STM_NAME), _args)                  \
+{                                                                \
+  int new_state = 0;                                             \
+  switch (state) {                                               \
+    __VA_ARGS__                                                  \
+      default:                                                   \
+    ABORT("%s: Got unknown state %d\n", __func__, state);        \
+    return -1;                                                   \
+  }                                                              \
+}
+#else
 #define STM(_args, _contents...)                                 \
 static STM_FUNC_DECL(STM_FUNC(STM_NAME), _args)                  \
 {                                                                \
@@ -159,9 +245,21 @@
     return -1;                                                   \
   }                                                              \
 }
+#endif
+
 
 #define UCT 1
 
+#ifndef __GNUC__
+#define STM_RUN(_state, _transitioned, _func, ...) \
+{                                                               \
+  int new_state = _func(*_state, 0/*no single step*/, __VA_ARGS__);    \
+  if (new_state > 0) {                                           \
+    *_state = new_state;                                         \
+    *_transitioned = TRUE;                                       \
+  }                                                              \
+}
+#else
 #define STM_RUN(_state, _transitioned, _func, _args...) \
 ({                                                               \
   int new_state = _func(*_state, 0/*no single step*/, _args);    \
@@ -170,11 +268,20 @@
     *_transitioned = TRUE;                                       \
   }                                                              \
 })
+#endif
 
+#ifndef __GNUC__
+#define STM_BEGIN(_state, _func, ...) \
+{                                                               \
+  *_state = _func(0/*BEGIN*/, 1/*single step*/, __VA_ARGS__);          \
+}
+#else
 #define STM_BEGIN(_state, _func, _args...) \
 ({                                                               \
   *_state = _func(0/*BEGIN*/, 1/*single step*/, _args);          \
 })
+#endif
+
 
 /************ End of Macros for state machine descriptions ***********/
 
@@ -188,14 +295,26 @@
 
 typedef uint bool;
 
-typedef struct _BridgeId {
+#ifdef __ICCARM__
+#pragma pack(1)
+#endif
+typedef COMPILER_INLINE_PACK struct _BridgeId {
   uchar bridge_identifier_priority[2];
   uchar bridge_address[6];
 } __attribute__((packed)) BridgeId;
-
-typedef struct {
+#ifdef __ICCARM__
+#pragma pack()
+#endif
+
+#ifdef __ICCARM__
+#pragma pack(1)
+#endif
+typedef COMPILER_INLINE_PACK struct _PathCost{
   uchar cost[4];
 } __attribute__((packed)) PathCost;
+#ifdef __ICCARM__
+#pragma pack()
+#endif
 
 static inline uint path_cost_to_uint(PathCost x)
 {
@@ -204,7 +323,22 @@
 
 }
 
+#ifndef __GNUC__
+static inline PathCost path_cost_add(PathCost x, uint y)
+{
+  PathCost r;
+
+  uint z = y +
+    (x.cost[0] << 24) + (x.cost[1] << 16) + (x.cost[2] << 8) + (x.cost[3]);
+
+  r.cost[0] = (z >> 24);
+  r.cost[1] = (z >> 16) & 0xff;
+  r.cost[2] = (z >> 8) & 0xff;
+  r.cost[3] = z & 0xff;
 
+  return r;
+}
+#else
 static inline PathCost path_cost_add(PathCost x, uint y)
 {
   uint z = y +
@@ -219,16 +353,27 @@
   return r;
 }
 
-typedef struct _PortId {
+#endif
+
+#ifdef __ICCARM__
+#pragma pack(1)
+#endif
+typedef COMPILER_INLINE_PACK struct _PortId {
   uchar port_id[2]; /* 4 high bits of priority and 12 for Id */
 } __attribute__((packed)) PortId;
+#ifdef __ICCARM__
+#pragma pack()
+#endif
 
 static inline uint port_id_to_uint(PortId p)
 {
   return (p.port_id[0] << 8) + p.port_id[1];
 }
 
-typedef struct _PriorityVector
+#ifdef __ICCARM__
+#pragma pack(1)
+#endif
+typedef COMPILER_INLINE_PACK struct _PriorityVector
 {
   BridgeId root_bridge_id;
   PathCost root_path_cost;
@@ -236,16 +381,24 @@
   PortId   designated_port_id;
   PortId   bridge_port_id;
 } __attribute__((packed)) PriorityVector;
-
+#ifdef __ICCARM__
+#pragma pack()
+#endif
+
+#ifdef __ICCARM__
+#pragma pack(1)
+#endif
 /* First 4 components of priority vector */
-typedef struct _PriorityVector4
+typedef COMPILER_INLINE_PACK struct _PriorityVector4
 {
   BridgeId root_bridge_id;
   PathCost root_path_cost;
   BridgeId designated_bridge_id;
   PortId   designated_port_id;
 } __attribute__((packed)) PriorityVector4;
-
+#ifdef __ICCARM__
+#pragma pack()
+#endif
 
 typedef struct _Times
 {
@@ -314,13 +467,18 @@
 
 #define STP_PROTOCOL_ID 0
 
-typedef struct _RawBPDU
+#ifdef __ICCARM__
+#pragma pack(1)
+#endif
+typedef COMPILER_INLINE_PACK struct _RawBPDU
 {
   uint16 protocol_id;
   uchar version;
   uchar type;
+#ifdef __GNUC__
   /* TCN has only upto this */
   uchar TCN_END[0];
+#endif
   uchar flags;
 #define BPDU_FLAG_TC                     0x1
 #define BPDU_FLAG_PROPOSAL               0x2
@@ -339,15 +497,26 @@
   uchar hello_time[2];
   uchar forward_delay[2];
   /* End of Config BPDU */
+#ifdef __GNUC__
   uchar CONFIG_END[0];
-
+#endif
   uchar version1_len;
   /* End of RST BPDU */
+#ifdef __GNUC__
   uchar RST_END[0];
-
+#endif
 } __attribute__((packed)) RawBPDU;
+#ifdef __ICCARM__
+#pragma pack()
+#endif
 
 
+#ifndef __GNU_C
+// Markers not allowed
+#define TCN_END_SIZE_BYTES    4
+#define CONFIG_END_SIZE_BYTES (sizeof(RawBPDU) - sizeof(uchar))
+#define RST_END_SIZE_BYTES    (sizeof(RawBPDU))
+#endif
 
 /* Values for adminPointToPointMAC */
 typedef enum _AdminP2P {
@@ -825,6 +994,20 @@
 /* PORT is not defined by this but is possible defined where this is used.
    FPORT iterates over all ports, so we need to use the FPORT->_field
    (with underscore) for for port variables in the expression here */
+#ifndef __GNUC__
+static bool g_ForAllPortBool;
+#define ForAllPort(result, expr)                                                     \
+{                                                                           \
+  Port *FPORT;                                                               \
+  bool res = TRUE;                                                           \
+  for (FPORT = BRIDGE->port_list; FPORT != NULL; FPORT = FPORT->port_next)   \
+    if (!(expr)) {                                                           \
+      res = FALSE;                                                           \
+      break;                                                                 \
+    }                                                                        \
+  result = res;                                                                       \
+}
+#else
 #define ForAllPort(expr)                                                     \
 ({                                                                           \
   Port *FPORT;                                                               \
@@ -836,17 +1019,27 @@
     }                                                                        \
   res;                                                                       \
 })
+#endif
 
 /* Execute statements for all ports */
 /* PORT is not defined by this but is possible defined where this is used.
    FPORT iterates over all ports, so we need to use the FPORT->_field
    (with underscore) for for port variables in statements here */
+#ifndef __GNUC__
+#define ForAllPortDo(...)                                          \
+{                                                                           \
+  Port *FPORT;                                                               \
+  for (FPORT = BRIDGE->port_list; FPORT != NULL; FPORT = FPORT->port_next)   \
+    { __VA_ARGS__ }                                                           \
+}
+#else
 #define ForAllPortDo(statements...)                                          \
 ({                                                                           \
   Port *FPORT;                                                               \
   for (FPORT = BRIDGE->port_list; FPORT != NULL; FPORT = FPORT->port_next)   \
     { statements }                                                           \
 })
+#endif
 
 // 17.20 ----- conditions and paramaters
 
@@ -857,7 +1050,7 @@
 #define AutoEdge AutoEdgePort
 
 // 17.20.3
-#define allSynced ForAllPort(FPORT->_synced || FPORT->_role == RootPort)
+#define allSynced(result) ForAllPort(result, FPORT->_synced || FPORT->_role == RootPort)
 
 // 17.20.4
 #define EdgeDelay (operPointToPointMAC?MigrateTime:MaxAge)
@@ -878,7 +1071,7 @@
 // #define MigrateTime MigrateTime
 
 // 17.20.10
-#define reRooted ForAllPort(FPORT == PORT || FPORT->_rrWhile == 0)
+#define reRooted(result) ForAllPort(result, FPORT == PORT || FPORT->_rrWhile == 0)
 
 // 17.20.11
 #define rstpVersion (ForceProtocolVersion >= 2)
@@ -968,10 +1161,15 @@
   BRIDGE->tcWhile_count+= change;
 }
 
+#ifndef __GNUC__
+#define set_tcWhile(_val) \
+{ uint _oldval = tcWhile; tcWhile = _val; \
+   update_tcWhile_count(BRIDGE_ARGS, (tcWhile?1:0) - (_oldval?1:0)); }
+#else
 #define set_tcWhile(_val) \
 ({ uint _oldval = tcWhile; tcWhile = _val; \
    update_tcWhile_count(BRIDGE_ARGS, (tcWhile?1:0) - (_oldval?1:0)); })
-
+#endif
 
 // 17.21.7
 static void newTcWhile(PORT_ARGS_PROTO)
@@ -1113,7 +1311,9 @@
 // 17.21.16
 static void setSelectedTree(BRIDGE_ARGS_PROTO)
 {
-  if (ForAllPort(FPORT->_reselect == FALSE))
+  bool temp;
+  ForAllPort(temp, FPORT->_reselect == FALSE);
+  if (temp)
     ForAllPortDo(
                  FPORT->_selected = TRUE;
                  );
@@ -1167,6 +1367,7 @@
 static void txConfig(PORT_ARGS_PROTO)
 {
   RawBPDU b;
+  int flags;
 
   b.protocol_id = STP_PROTOCOL_ID;
   b.version = 0;
@@ -1174,7 +1375,7 @@
 
   b.priority = designatedPriority;
 
-  int flags = 0;
+  flags = 0;
   flags |= (tcWhile != 0) ? BPDU_FLAG_TC : 0;
   flags |= (tcAck) ? BPDU_FLAG_TC_ACK : 0;
   b.flags = flags;
@@ -1182,13 +1383,19 @@
   set_bpdu_times(&b, &designatedTimes);
 
   DEBUG("Sending Config BPDU\n");
+#ifndef __GNUC__
+  STP_OUT_tx_bpdu(PORT->user_ref, &b, CONFIG_END_SIZE_BYTES);
+#else
   STP_OUT_tx_bpdu(PORT->user_ref, &b, offsetof(RawBPDU, CONFIG_END));
+#endif
 }
 
 // 17.21.20
 static void txRstp(PORT_ARGS_PROTO)
 {
   RawBPDU b;
+  int flags;
+  int r;
 
   b.protocol_id = STP_PROTOCOL_ID;
   b.version = 2;
@@ -1196,9 +1403,9 @@
 
   b.priority = designatedPriority;
 
-  int flags = 0;
+  flags = 0;
 
-  int r = role;
+  r = role;
   if (r == AlternatePort || r == BackupPort)
     r = AltBackupPort;
 
@@ -1218,7 +1425,11 @@
   b.version1_len = 0;
 
   DEBUG("Sending RST BPDU\n");
+#ifndef __GNUC__
+  STP_OUT_tx_bpdu(PORT->user_ref, &b, RST_END_SIZE_BYTES);
+#else
   STP_OUT_tx_bpdu(PORT->user_ref, &b, offsetof(RawBPDU, RST_END));
+#endif
 }
 
 // 17.21.21
@@ -1231,7 +1442,12 @@
   b.type = BPDUTypeTCN;
 
   DEBUG("Sending TCN BPDU\n");
+#ifndef __GNUC__
+  STP_OUT_tx_bpdu(PORT->user_ref, &b, TCN_END_SIZE_BYTES);
+#else
   STP_OUT_tx_bpdu(PORT->user_ref, &b, offsetof(RawBPDU, TCN_END));
+#endif
+
 }
 
 // 17.21.22
@@ -1419,7 +1635,7 @@
 
 STM(PORT_ARGS_PROTO,
 
-    GC();
+    GC(;);
 
     BG(
        TR(UCT, ONE_SECOND);
@@ -1502,7 +1718,7 @@
 
 STM(PORT_ARGS_PROTO,
 
-    GC();
+    GC(;);
 
     BG(
        TR(UCT, CHECKING_RSTP);
@@ -1546,7 +1762,7 @@
 
 STM(PORT_ARGS_PROTO,
 
-    GC();
+    GC(;);
 
     BG(
        TR(AdminEdge, EDGE);
@@ -1586,7 +1802,7 @@
 
 STM(PORT_ARGS_PROTO,
 
-    GC();
+    GC(;);
 
     BG(
        TR(UCT, TRANSMIT_INIT);
@@ -1772,7 +1988,7 @@
 
 STM(BRIDGE_ARGS_PROTO,
 
-    GC();
+    GC(;);
 
     BG(
        TR(UCT, INIT_BRIDGE);
@@ -1789,7 +2005,8 @@
        updtRolesTree(BRIDGE_ARGS);
        setSelectedTree(BRIDGE_ARGS);
        ,
-       TR(!ForAllPort(!FPORT->_reselect), ROLE_SELECTION);
+       ForAllPort(g_ForAllPortBool,!FPORT->_reselect);
+       TR(!g_ForAllPortBool, ROLE_SELECTION);
        );
 
     );
@@ -1935,14 +2152,17 @@
        rrWhile = FwdDelay;
        ,
        TR(proposed && !agree && xc, ROOT_PROPOSED);
-       TR(((allSynced && !agree) || (proposed && agree)) && xc, ROOT_AGREED);
+       allSynced(g_ForAllPortBool);
+       TR(((g_ForAllPortBool && !agree) || (proposed && agree)) && xc, ROOT_AGREED);
        TR(((agreed && !synced) || (sync && synced)) && xc, ROOT_SYNCED);
        TR(!forward && !reRoot && xc, REROOT);
        TR(reRoot && forward && xc, REROOTED);
+       reRooted(g_ForAllPortBool);
        TR((fdWhile == 0 ||
-           ((reRooted && (rbWhile == 0)) && (rstpVersion))) && !learn && xc,
+           ((g_ForAllPortBool && (rbWhile == 0)) && (rstpVersion))) && !learn && xc,
           ROOT_LEARN);
-       TR((fdWhile == 0 || ((reRooted && (rbWhile == 0)) && (rstpVersion)))
+       reRooted(g_ForAllPortBool);
+       TR((fdWhile == 0 || ((g_ForAllPortBool && (rbWhile == 0)) && (rstpVersion)))
           && learn && !forward && xc,
           ROOT_FORWARD);
        TR(rrWhile != FwdDelay && xc, ROOT_PORT);
@@ -2058,7 +2278,8 @@
        ,
        TR(proposed && !agree && xc,
           ALTERNATE_PROPOSED);
-       TR(((allSynced && !agree) || (proposed && agree)) && xc,
+       allSynced(g_ForAllPortBool);
+       TR(((g_ForAllPortBool && !agree) || (proposed && agree)) && xc,
           ALTERNATE_AGREED);
        TR((rbWhile != 2*HelloTime) && (role == BackupPort) && xc,
           BACKUP_PORT);
@@ -2080,7 +2301,7 @@
 
 STM(PORT_ARGS_PROTO,
 
-    GC();
+    GC(;);
 
     BG(
        TR(UCT, DISCARDING);
@@ -2125,7 +2346,7 @@
 
 STM(PORT_ARGS_PROTO,
 
-    GC();
+    GC(;);
 
     BG(
        TR(UCT, INACTIVE);
@@ -2248,11 +2469,12 @@
 
 static void state_machines_run(BRIDGE_ARGS_PROTO)
 {
+  int i;
+  bool transitioned;
+
   if (!BRIDGE->stp_on)
     return;
 
-  int i;
-  bool transitioned;
   do {
     transitioned = FALSE;
     for (i = 0; i < NUM_BRIDGE_STATE_MACHINES; i++)
@@ -2268,10 +2490,10 @@
 
 static void state_machines_begin(BRIDGE_ARGS_PROTO)
 {
+  int i;
   if (!BRIDGE->stp_on)
     return;
 
-  int i;
   for (i = 0; i < NUM_BRIDGE_STATE_MACHINES; i++)
     STM_BEGIN(&BRIDGE->state[i], bridge_stms[i], BRIDGE_ARGS);
 
@@ -2520,6 +2742,9 @@
 int STP_IN_set_bridge_config(Bridge *BRIDGE, const STP_BridgeConfig *cfg)
 {
   int r = 0;
+  Times new_times;
+  bool set_times;
+
   bool changed = FALSE, init = FALSE;
   /* First, validation */
 
@@ -2540,8 +2765,8 @@
     }
   }
 
-  Times new_times = BridgeTimes;
-  bool set_times = FALSE;
+  new_times = BridgeTimes;
+  set_times = FALSE;
 
   if (cfg->set_bridge_hello_time) {
     // 17.14 - Table 17-1
@@ -2782,6 +3007,16 @@
 typedef STP_BridgeConfig  STP_bridgeConfig;
 typedef STP_PortConfig    STP_portConfig;
 
+#ifndef __GNUC__
+#define SET_CFG(_type, _arg, _field, _value, _retval) \
+{                                                           \
+  STP_ ## _type ## Config cfg;                               \
+  ZERO(cfg);                                                 \
+  cfg._type ## _ ## _field = _value;                         \
+  cfg. set_ ## _type ## _ ## _field = 1;                     \
+  _retval = STP_IN_set_ ## _type ## _config(_arg, &cfg);               \
+}
+#else
 #define SET_CFG(_type, _arg, _field, _value) \
 ({                                                           \
   STP_ ## _type ## Config cfg;                               \
@@ -2790,44 +3025,59 @@
   cfg. set_ ## _type ## _ ## _field = 1;                     \
   STP_IN_set_ ## _type ## _config(_arg, &cfg);               \
 })
+#endif
 
 /* Single field Bridge config functions */
 
 int STP_IN_set_protocol_version(Bridge *BRIDGE, unsigned int version)
 {
-  return SET_CFG(bridge, BRIDGE, protocol_version, version);
+  bool retval;
+  SET_CFG(bridge, BRIDGE, protocol_version, version, retval);
+  return retval;
 }
 
 int STP_IN_set_bridge_address(Bridge *BRIDGE, const STP_MacAddress *addr)
 {
-  return SET_CFG(bridge, BRIDGE, address, *addr);
+  bool retval;
+  SET_CFG(bridge, BRIDGE, address, *addr, retval);
+  return retval;
 }
 
 int STP_IN_set_bridge_priority(Bridge *BRIDGE, unsigned int priority)
 {
-  return SET_CFG(bridge, BRIDGE, priority, priority);
+  bool retval;
+  SET_CFG(bridge, BRIDGE, priority, priority, retval);
+  return retval;
 }
 
 int STP_IN_set_bridge_hello_time(Bridge *BRIDGE, unsigned int hello_time)
 {
-  return SET_CFG(bridge, BRIDGE, hello_time, hello_time);
+  bool retval;
+  SET_CFG(bridge, BRIDGE, hello_time, hello_time, retval);
+  return retval;
 }
 
 int STP_IN_set_bridge_max_age(Bridge *BRIDGE, unsigned int max_age)
 {
-  return SET_CFG(bridge, BRIDGE, max_age, max_age);
+  bool retval;
+  SET_CFG(bridge, BRIDGE, max_age, max_age, retval);
+  return retval;
 }
 
 
 int STP_IN_set_bridge_forward_delay(Bridge *BRIDGE, unsigned int fwd_delay)
 {
-  return SET_CFG(bridge, BRIDGE, forward_delay, fwd_delay);
+  bool retval;
+  SET_CFG(bridge, BRIDGE, forward_delay, fwd_delay, retval);
+  return retval;
 }
 
 
 int STP_IN_set_tx_hold_count(Bridge *BRIDGE, unsigned int count)
 {
-  return SET_CFG(bridge, BRIDGE, tx_hold_count, count);
+  bool retval;
+  SET_CFG(bridge, BRIDGE, tx_hold_count, count, retval);
+  return retval;
 }
 
 
@@ -2835,28 +3085,38 @@
 
 int STP_IN_set_port_priority(Port *PORT, unsigned int priority)
 {
-  return SET_CFG(port, PORT, priority, priority);
+  bool retval;
+  SET_CFG(port, PORT, priority, priority, retval);
+  return retval;
 }
 
 int STP_IN_set_port_pathcost(Port *PORT, unsigned int pcost)
 {
-  return SET_CFG(port, PORT, pathcost, pcost);
+  bool retval;
+  SET_CFG(port, PORT, pathcost, pcost, retval);
+  return retval;
 }
 
 /* edge is 0 or 1 */
 int STP_IN_set_port_admin_edge(Port *PORT, unsigned int edge)
 {
-  return SET_CFG(port, PORT, admin_edge, edge);
+  bool retval;
+  SET_CFG(port, PORT, admin_edge, edge, retval);
+  return retval;
 }
 
 int STP_IN_set_port_auto_edge(Port *PORT, unsigned int edge)
 {
-  return SET_CFG(port, PORT, auto_edge, edge);
+  bool retval;
+  SET_CFG(port, PORT, auto_edge, edge, retval);
+  return retval;
 }
 
 int STP_IN_set_port_admin_p2p(Port *PORT, unsigned int p2p)
 {
-  return SET_CFG(port, PORT, admin_p2p, p2p);
+  bool retval;
+  SET_CFG(port, PORT, admin_p2p, p2p, retval);
+  return retval;
 }
 
 /* Force migration check */
@@ -2957,6 +3217,7 @@
  */
 Bridge *STP_IN_bridge_create(void *user_ref)
 {
+  Bridge *BRIDGE;
   Bridge *b = STP_OUT_mem_zalloc(sizeof(Bridge));
 
   if (!b) {
@@ -2966,7 +3227,7 @@
 
   b->user_ref = user_ref;
 
-  Bridge *BRIDGE = b;
+  BRIDGE = b;
 
   /* Default configuration values */
   BridgeIdentifierPriority[0] = 0x80; // Default bridge priority = 32768
@@ -3003,12 +3264,15 @@
 Port *STP_IN_port_create(Bridge *BRIDGE,
                          unsigned int port_number, void *user_ref)
 {
+  Port *p;
+  Port *PORT;
+
   if (port_number == 0 || (port_number & ~0x0fff)) {
     ERROR("Port id should be in the range 1 - 1023\n");
     return NULL;
   }
 
-  Port *p = STP_OUT_mem_zalloc(sizeof(Port));
+  p = STP_OUT_mem_zalloc(sizeof(Port));
 
   if (!p) {
     ERROR("Couldn't allocate memory for port\n");
@@ -3021,7 +3285,7 @@
   p->port_next = BRIDGE->port_list;
   BRIDGE->port_list = p;
 
-  Port *PORT = p;
+  PORT = p;
 
   portId.port_id[0] = 0x80 | (port_number >> 8); // Default port priority = 128
   portId.port_id[1] = port_number & 0xff;
@@ -3048,6 +3312,7 @@
 void STP_IN_port_delete(Port *PORT)
 {
   Bridge *BRIDGE = PORT->bridge;
+  Port **prev; 
 
   /* Disable */
   if (portEnabled) {
@@ -3056,7 +3321,7 @@
   }
 
   /* Find position in list */
-  Port **prev = &BRIDGE->port_list;
+  prev = &BRIDGE->port_list;
   while (*prev != PORT && *prev != NULL)
     prev = &(*prev)->port_next;
 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Bridge] Update to RSTP lib
  2009-01-12 20:42   ` Manish Talreja
@ 2009-01-12 21:32     ` Stephen Hemminger
  0 siblings, 0 replies; 6+ messages in thread
From: Stephen Hemminger @ 2009-01-12 21:32 UTC (permalink / raw)
  To: Manish Talreja; +Cc: bridge@lists.linux-foundation.org

On Mon, 12 Jan 2009 15:42:11 -0500
Manish Talreja <mtalreja@crestron.com> wrote:

> Here are the unified diff files as requested.
> 
> You are right about #pragma pack(). That's supported on pretty much all compilers. However, __attribute__ is not supported on all compilers and that's what was used in the original code. Our goal was to leave the gcc syntax the way it was in the original code. If you are ok with getting rid of the __attribute__((packed)), the code will be a lot more readable.
> 
> -Manish
> 
> 
> -----Original Message-----
> From: richardvoigt@gmail.com [mailto:richardvoigt@gmail.com]
> Sent: Monday, January 12, 2009 3:01 PM
> To: Manish Talreja
> Cc: bridge@lists.linux-foundation.org
> Subject: Re: [Bridge] Update to RSTP lib
> 
> If you want your patch to be understandable, create it as a contextual
> diff with the -u option.
> 
> Do the pack pragmas really need to be conditional?  Pretty much every
> compiler will do the right thing on encountering those.  (Using
> __attribute__ as well, for compilers that support it, isn't a bad
> thing but no need to avoid #pragma pack on gcc).
> 
> On Mon, Jan 12, 2009 at 1:22 PM, Manish Talreja <mtalreja@crestron.com> wrote:
> > We have been using the RSTP implementation posted at this location:
> >
> > https://lists.linux-foundation.org/pipermail/bridge/2008-May/005859.html
> >
> >
> >
> > We have modified rstp.c and rstp.h to work with compilers other than gcc.
> > Attached are two patch files that show our changes. Currently, we have
> > tested this code with the Diab compiler (v 4.4a) and with IAR compiler (v
> > 5.11).
> >

I would rather the code be cleaned out of the ugly macro's ifdef's.
Also only use pack if necessary (ie unaligned struct like:
      struct {
           char a;
	   unsigned b;
       };
If all fields are correct types to be naturally packed, then pack
is unnecessary. Compiler's aren't smart enough to recognize this
and assume misalignment so they will generate bloated code for anything
with #pragma pack


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Bridge] Update to RSTP lib
@ 2012-12-21 16:31 Christopher Merck
  0 siblings, 0 replies; 6+ messages in thread
From: Christopher Merck @ 2012-12-21 16:31 UTC (permalink / raw)
  To: bridge@lists.linux-foundation.org

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

> > On Mon, Jan 12, 2009 at 1:22 PM, Manish Talreja <mtalreja at crestron.com> wrote:
> > > We have been using the RSTP implementation posted at this location:
> > >
> > > https://lists.linux-foundation.org/pipermail/bridge/2008-May/005859.html
> > >
> > >
> > >
> > > We have modified rstp.c and rstp.h to work with compilers other than gcc.
> > > Attached are two patch files that show our changes. Currently, we have
> > > tested this code with the Diab compiler (v 4.4a) and with IAR compiler (v
> > > 5.11).
> > >

We have further modified rstp.c and rstp.h to work with more compilers. The attached unified diff shows the changes. This has currently been tested with VC++ 2008.


This e-mail message and all attachments transmitted with it may contain legally privileged and confidential information intended solely for the use of the addressee. If you are not the intended recipient, you are hereby notified that any reading, dissemination, distribution, copying, or other use of this message or its attachments is strictly prohibited.

[-- Attachment #2: rstp.patch --]
[-- Type: application/octet-stream, Size: 16333 bytes --]

--- rstp.h.orig	2012-12-21 09:24:35.492806900 -0500
+++ rstp.h	2012-12-21 09:24:45.710741400 -0500
@@ -222,6 +222,7 @@
 void *STP_OUT_mem_zalloc(unsigned int size);
 
 void STP_OUT_mem_free(void *);
+int STP_OUT_topologychanged(int setOrRead);
 
 
 
@@ -232,6 +233,10 @@
 #define COMPILER_INLINE_PACK  __packed__
 #elif defined (__ICCARM__)
 #define COMPILER_INLINE_PACK
+#elif defined WIN32
+#define COMPILER_INLINE_PACK
+#elif defined WINCE
+#define COMPILER_INLINE_PACK
 #else
 DEFINE PACKING FOR YOUR COMPILER
 #endif
--- rstp.c.orig	2012-12-21 09:27:38.374409000 -0500
+++ rstp.c	2012-12-21 09:27:53.053914900 -0500
@@ -28,11 +28,21 @@
   Section references in the code refer to those parts of the standard.
 *********************************************************************/
 
+/* Changes: 
+23 MAY 12: 
+    renamed WINCE_BUILD, STP_DEBUG_PRINT, STP_ERROR_PRINT, and STP_ABORT macros
+    added STP_ERROR_PRINT_NO_REFS macro to avoid dereference of uninitialized pointers (bug in original code)
+*/
+
 #include "rstp.h"
 #include <string.h>
 
-/* Macros to be used in the later parts */
+#ifdef WINCE_BUILD
+    #pragma warning(disable: 4127 4244 4100 4389 4245 4700)
+    #define inline
+#endif
 
+/* Macros to be used in the later parts */
 
 #ifndef NULL
 #define NULL ((void *)0)
@@ -107,19 +117,21 @@
 
 /* Logging macros */
 #ifndef __GNUC__
-#define DEBUG(...) \
+#define STP_DEBUG_PRINT(...) \
   STP_OUT_logmsg(BRIDGE->user_ref, PORT->user_ref, STP_LOG_LEVEL_DEBUG, __VA_ARGS__)
-#define ERROR(...) \
+#define STP_ERROR_PRINT(...) \
   STP_OUT_logmsg(BRIDGE->user_ref, PORT->user_ref, STP_LOG_LEVEL_ERROR, __VA_ARGS__)
-#define ABORT(...) \
-  { ERROR(__VA_ARGS__);}
+#define STP_ERROR_PRINT_NO_REFS(...) \
+  STP_OUT_logmsg(NULL, NULL, STP_LOG_LEVEL_ERROR, __VA_ARGS__)
+#define STP_ABORT(...) \
+  { STP_ERROR_PRINT(__VA_ARGS__);}
 #else
-#define DEBUG(fmt...) \
+#define STP_DEBUG_PRINT(fmt...) \
   STP_OUT_logmsg(BRIDGE->user_ref, PORT->user_ref, STP_LOG_LEVEL_DEBUG, fmt)
-#define ERROR(fmt...) \
+#define STP_ERROR_PRINT(fmt...) \
   STP_OUT_logmsg(BRIDGE->user_ref, PORT->user_ref, STP_LOG_LEVEL_ERROR, fmt)
-#define ABORT(fmt...) \
-  ({ ERROR(fmt); *(int *)0 = 1; })
+#define STP_ABORT(fmt...) \
+  ({ STP_ERROR_PRINT(fmt); *(int *)0 = 1; })
 #endif
 
 /***************** Macros for state machine descriptions ********************/
@@ -151,7 +163,7 @@
 {                                                                           \
   if (_cond) {                                                               \
     new_state = STN(_new_state);                                             \
-    DEBUG("%s: Transition to state %s\n", _STR(STM_NAME), #_new_state);      \
+    STP_DEBUG_PRINT("%s: Transition to state %s\n", _STR(STM_NAME), #_new_state);      \
     goto STM_LABEL(_new_state);                                              \
   }                                                                          \
 }
@@ -160,7 +172,7 @@
 ({                                                                           \
   if (_cond) {                                                               \
     new_state = STN(_new_state);                                             \
-    DEBUG("%s: Transition to state %s\n", _STR(STM_NAME), #_new_state);      \
+    STP_DEBUG_PRINT("%s: Transition to state %s\n", _STR(STM_NAME), #_new_state);      \
     goto STM_LABEL(_new_state);                                              \
   }                                                                          \
 })
@@ -189,13 +201,13 @@
 #define BG(...)                                              \
          case STN(BEGIN):                                                \
              __VA_ARGS__                                                \
-             ABORT("%s: No where to go from BEGIN\n", _STR(STM_NAME));   \
+             STP_ABORT("%s: No where to go from BEGIN\n", _STR(STM_NAME));   \
              return -1
 #else
 #define BG(_transitions...)                                              \
          case STN(BEGIN):                                                \
              _transitions                                                \
-             ABORT("%s: No where to go from BEGIN\n", _STR(STM_NAME));   \
+             STP_ABORT("%s: No where to go from BEGIN\n", _STR(STM_NAME));   \
              return -1
 #endif
 
@@ -221,6 +233,12 @@
 /* This just barely works. _args expansion includes commas */
 //int STM_FUNC(STM_NAME)(int state, int single_step, _args)
 
+#ifdef WINCE_BUILD
+    #define FUNC __FUNCTION__
+#else
+    #define FUNC __func__
+#endif
+
 #ifndef __GNUC__
 #define STM(_args, ...)                                 \
 static STM_FUNC_DECL(STM_FUNC(STM_NAME), _args)                  \
@@ -229,7 +247,7 @@
   switch (state) {                                               \
     __VA_ARGS__                                                  \
       default:                                                   \
-    ABORT("%s: Got unknown state %d\n", __func__, state);        \
+    STP_ABORT("%s: Got unknown state %d\n", FUNC, state);        \
     return -1;                                                   \
   }                                                              \
 }
@@ -241,7 +259,7 @@
   switch (state) {                                               \
     _contents                                                    \
       default:                                                   \
-    ABORT("%s: Got unknown state %d\n", __func__, state);        \
+    STP_ABORT("%s: Got unknown state %d\n", FUNC, state);        \
     return -1;                                                   \
   }                                                              \
 }
@@ -295,32 +313,31 @@
 
 typedef uint bool;
 
-#ifdef __ICCARM__
-#pragma pack(1)
-#endif
+#if defined(__ICCARM__) || defined(WINCE_BUILD)
+#pragma pack(1)
+#endif
 typedef COMPILER_INLINE_PACK struct _BridgeId {
   uchar bridge_identifier_priority[2];
   uchar bridge_address[6];
 } __attribute__((packed)) BridgeId;
-#ifdef __ICCARM__
-#pragma pack()
-#endif
-
-#ifdef __ICCARM__
-#pragma pack(1)
-#endif
+#if defined(__ICCARM__) || defined(WINCE_BUILD)
+#pragma pack()
+#endif
+
+#if defined(__ICCARM__) || defined(WINCE_BUILD)
+#pragma pack(1)
+#endif
 typedef COMPILER_INLINE_PACK struct _PathCost{
   uchar cost[4];
 } __attribute__((packed)) PathCost;
-#ifdef __ICCARM__
-#pragma pack()
-#endif
+#if defined(__ICCARM__) || defined(WINCE_BUILD)
+#pragma pack()
+#endif
 
 static inline uint path_cost_to_uint(PathCost x)
 {
   return
     (x.cost[0] << 24) + (x.cost[1] << 16) + (x.cost[2] << 8) + (x.cost[3]);
-
 }
 
 #ifndef __GNUC__
@@ -355,24 +372,24 @@
 
 #endif
 
-#ifdef __ICCARM__
-#pragma pack(1)
-#endif
+#if defined(__ICCARM__) || defined(WINCE_BUILD)
+#pragma pack(1)
+#endif
 typedef COMPILER_INLINE_PACK struct _PortId {
   uchar port_id[2]; /* 4 high bits of priority and 12 for Id */
 } __attribute__((packed)) PortId;
-#ifdef __ICCARM__
-#pragma pack()
-#endif
+#if defined(__ICCARM__) || defined(WINCE_BUILD)
+#pragma pack()
+#endif
 
 static inline uint port_id_to_uint(PortId p)
 {
   return (p.port_id[0] << 8) + p.port_id[1];
 }
 
-#ifdef __ICCARM__
-#pragma pack(1)
-#endif
+#if defined(__ICCARM__) || defined(WINCE_BUILD)
+#pragma pack(1)
+#endif
 typedef COMPILER_INLINE_PACK struct _PriorityVector
 {
   BridgeId root_bridge_id;
@@ -381,13 +398,13 @@
   PortId   designated_port_id;
   PortId   bridge_port_id;
 } __attribute__((packed)) PriorityVector;
-#ifdef __ICCARM__
-#pragma pack()
-#endif
-
-#ifdef __ICCARM__
-#pragma pack(1)
-#endif
+#if defined(__ICCARM__) || defined(WINCE_BUILD)
+#pragma pack()
+#endif
+
+#if defined(__ICCARM__) || defined(WINCE_BUILD)
+#pragma pack(1)
+#endif
 /* First 4 components of priority vector */
 typedef COMPILER_INLINE_PACK struct _PriorityVector4
 {
@@ -396,9 +413,9 @@
   BridgeId designated_bridge_id;
   PortId   designated_port_id;
 } __attribute__((packed)) PriorityVector4;
-#ifdef __ICCARM__
-#pragma pack()
-#endif
+#if defined(__ICCARM__) || defined(WINCE_BUILD)
+#pragma pack()
+#endif
 
 typedef struct _Times
 {
@@ -467,9 +484,9 @@
 
 #define STP_PROTOCOL_ID 0
 
-#ifdef __ICCARM__
-#pragma pack(1)
-#endif
+#if defined(__ICCARM__) || defined(WINCE_BUILD)
+#pragma pack(1)
+#endif
 typedef COMPILER_INLINE_PACK struct _RawBPDU
 {
   uint16 protocol_id;
@@ -506,9 +523,9 @@
   uchar RST_END[0];
 #endif
 } __attribute__((packed)) RawBPDU;
-#ifdef __ICCARM__
-#pragma pack()
-#endif
+#if defined(__ICCARM__) || defined(WINCE_BUILD)
+#pragma pack()
+#endif
 
 
 #ifndef __GNU_C
@@ -1156,6 +1173,7 @@
     /* Becoming nonzero from zero */
     BRIDGE->tc_count++;
     BRIDGE->time_since_tc = 0;
+    STP_OUT_topologychanged(1);
   }
 
   BRIDGE->tcWhile_count+= change;
@@ -1382,7 +1400,7 @@
 
   set_bpdu_times(&b, &designatedTimes);
 
-  DEBUG("Sending Config BPDU\n");
+  STP_DEBUG_PRINT("Sending Config BPDU\n");
 #ifndef __GNUC__
   STP_OUT_tx_bpdu(PORT->user_ref, &b, CONFIG_END_SIZE_BYTES);
 #else
@@ -1424,7 +1442,7 @@
 
   b.version1_len = 0;
 
-  DEBUG("Sending RST BPDU\n");
+  STP_DEBUG_PRINT("Sending RST BPDU\n");
 #ifndef __GNUC__
   STP_OUT_tx_bpdu(PORT->user_ref, &b, RST_END_SIZE_BYTES);
 #else
@@ -1441,7 +1459,7 @@
   b.version = 0;
   b.type = BPDUTypeTCN;
 
-  DEBUG("Sending TCN BPDU\n");
+  STP_DEBUG_PRINT("Sending TCN BPDU\n");
 #ifndef __GNUC__
   STP_OUT_tx_bpdu(PORT->user_ref, &b, TCN_END_SIZE_BYTES);
 #else
@@ -1594,9 +1612,9 @@
                  }
                  break;
                default:
-                 ERROR("Unknown value for infoIs: %d\n", infoIs);
+                 STP_ERROR_PRINT("Unknown value for infoIs: %d\n", infoIs);
                }
-                 DEBUG("Selected Role Set to: %d\n", selectedRole);
+                 STP_DEBUG_PRINT("Selected Role Set to: %d\n", selectedRole);
                );
 }
 
@@ -2642,7 +2660,7 @@
     return; // No action - else we might fail the next test
 
   if (rcvdBpdu) {
-    ABORT("Port hasn't processed old BPDU\n");
+    STP_ABORT("Port hasn't processed old BPDU\n");
     return;
   }
 
@@ -2652,7 +2670,7 @@
   if (p->protocol_id != STP_PROTOCOL_ID)
     return;
 
-  DEBUG("Received BPDU of type %d\n", p->type);
+  STP_DEBUG_PRINT("Received BPDU of type %d\n", p->type);
 
   if (// 9.3.4 a)
       (p->type == BPDUTypeConfig &&
@@ -2673,7 +2691,7 @@
     /* BPDU has been validated */
   }
   else {
-    DEBUG("BPDU validation failed\n");
+    STP_DEBUG_PRINT("BPDU validation failed\n");
     return;
   }
 
@@ -2723,7 +2741,7 @@
 static int check_times(BRIDGE_ARGS_PROTO, int max_age, int fwd_delay)
 {
   if (2*(fwd_delay - 1) < max_age) {
-    ERROR("Configured BridgeTimes doesn't meet "
+    STP_ERROR_PRINT("Configured BridgeTimes doesn't meet "
           "2 * (Bridge Foward Delay - 1) >= Bridge Max Age\n");
     return -1;
   }
@@ -2731,7 +2749,7 @@
   // This condition never fails, with hello_time == 2 && max_age >= 6.
   // So no need to check.
   //if (max_age < 2*(BridgeHelloTime + 1)) {
-  //  ERROR("Doesn't meet Bridge Max Age >= 2 * (Bridge Hello Time + 1) ");
+  //  STP_ERROR_PRINT("Doesn't meet Bridge Max Age >= 2 * (Bridge Hello Time + 1) ");
   //  return -1;
   //}
 
@@ -2750,7 +2768,7 @@
 
   if (cfg->set_bridge_protocol_version) {
     if (cfg->bridge_protocol_version != 0 && cfg->bridge_protocol_version != 2) {
-      ERROR("Protocol version must be 0 or 2\n");
+      STP_ERROR_PRINT("Protocol version must be 0 or 2\n");
       r = -1;
     }
   }
@@ -2760,7 +2778,7 @@
   if (cfg->set_bridge_priority) {
     // 17.14 - Table 17-2
     if (cfg->bridge_priority & ~0xf000) {
-      ERROR("Bridge Priority restricted to 0-61440 in steps of 4096\n");
+      STP_ERROR_PRINT("Bridge Priority restricted to 0-61440 in steps of 4096\n");
       r = -1;
     }
   }
@@ -2771,7 +2789,7 @@
   if (cfg->set_bridge_hello_time) {
     // 17.14 - Table 17-1
     if (cfg->bridge_hello_time != BridgeHelloTime) {
-      ERROR("Hello Time cannot be changed from 2s in RSTP\n");
+      STP_ERROR_PRINT("Hello Time cannot be changed from 2s in RSTP\n");
       r = -1;
     }
     new_times.hello_time = cfg->bridge_hello_time;
@@ -2781,7 +2799,7 @@
   if (cfg->set_bridge_max_age) {
     // 17.14 - Table 17-1
     if (cfg->bridge_max_age < 6 || cfg->bridge_max_age > 40) {
-      ERROR("Bridge Max Age must be between 6 and 40\n");
+      STP_ERROR_PRINT("Bridge Max Age must be between 6 and 40\n");
       r = -1;
     }
     new_times.max_age = cfg->bridge_max_age;
@@ -2791,7 +2809,7 @@
   if (cfg->set_bridge_forward_delay) {
     // 17.14 - Table 17-1
     if (cfg->bridge_forward_delay < 4 || cfg->bridge_forward_delay > 30) {
-      ERROR("Bridge Forward Delay must be between 4 and 30\n");
+      STP_ERROR_PRINT("Bridge Forward Delay must be between 4 and 30\n");
       r = -1;
     }
     new_times.forward_delay = cfg->bridge_forward_delay;
@@ -2804,7 +2822,7 @@
   if (cfg->set_bridge_tx_hold_count) {
     // 17.14 - Table 17-1
     if (cfg->bridge_tx_hold_count < 1 || cfg->bridge_tx_hold_count > 10) {
-      ERROR("Transmit Hold Count must be between 1 and 10\n");
+      STP_ERROR_PRINT("Transmit Hold Count must be between 1 and 10\n");
       r = -1;
     }
   }
@@ -2906,7 +2924,7 @@
   if (cfg->set_port_priority) {
     // 17.14 - Table 17-2
     if (cfg->port_priority & ~0xf0) {
-      ERROR("Port Priority restricted to 0-240 in steps of 16\n");
+      STP_ERROR_PRINT("Port Priority restricted to 0-240 in steps of 16\n");
       r = -1;
     }
   }
@@ -2914,28 +2932,28 @@
   if (cfg->set_port_pathcost) {
     // 17.14 - Note 3 and Table 17-3
     if (cfg->port_pathcost > 200000000) {
-      ERROR("Port path cost restricted to 1-200,000,000 or 0 for auto\n");
+      STP_ERROR_PRINT("Port path cost restricted to 1-200,000,000 or 0 for auto\n");
       r = -1;
     }
   }
 
   if (cfg->set_port_admin_edge) {
     if (cfg->port_admin_edge != 0 && cfg->port_admin_edge != 1) {
-      ERROR("Admin Edge must be 0 or 1\n");
+      STP_ERROR_PRINT("Admin Edge must be 0 or 1\n");
       r = -1;
     }
   }
 
   if (cfg->set_port_auto_edge) {
     if (cfg->port_auto_edge != 0 && cfg->port_auto_edge != 1) {
-      ERROR("Auto Edge must be 0 or 1\n");
+      STP_ERROR_PRINT("Auto Edge must be 0 or 1\n");
       r = -1;
     }
   }
 
   if (cfg->set_port_admin_p2p) {
     if (cfg->port_admin_p2p < 0 || cfg->port_admin_p2p > 2) {
-      ERROR("Admin P2P must be "
+      STP_ERROR_PRINT("Admin P2P must be "
             "0 (force false), 1 (force true), or 2 (auto)\n");
       r = -1;
     }
@@ -3126,7 +3144,7 @@
   Bridge *BRIDGE = PORT->bridge;
 
   if (!BRIDGE->stp_on) {
-    ERROR("Bridge is down\n");
+    STP_ERROR_PRINT("Bridge is down\n");
     return -1;
   }
 
@@ -3221,7 +3239,7 @@
   Bridge *b = STP_OUT_mem_zalloc(sizeof(Bridge));
 
   if (!b) {
-    ERROR("Couldn't allocate memory for bridge\n");
+    STP_ERROR_PRINT_NO_REFS("Couldn't allocate memory for bridge\n");
     return NULL;
   }
 
@@ -3268,14 +3286,14 @@
   Port *PORT;
 
   if (port_number == 0 || (port_number & ~0x0fff)) {
-    ERROR("Port id should be in the range 1 - 1023\n");
+    STP_ERROR_PRINT_NO_REFS("Port id should be in the range 1 - 1023\n");
     return NULL;
   }
 
   p = STP_OUT_mem_zalloc(sizeof(Port));
 
   if (!p) {
-    ERROR("Couldn't allocate memory for port\n");
+    STP_ERROR_PRINT_NO_REFS("Couldn't allocate memory for port\n");
     return NULL;
   }
 
@@ -3312,7 +3330,7 @@
 void STP_IN_port_delete(Port *PORT)
 {
   Bridge *BRIDGE = PORT->bridge;
-  Port **prev; 
+  Port **prev;
 
   /* Disable */
   if (portEnabled) {
@@ -3326,7 +3344,7 @@
     prev = &(*prev)->port_next;
 
   if (*prev == NULL)
-    ABORT("port not in its bridge's list\n");
+    STP_ABORT("port not in its bridge's list\n");
 
   /* Remove from list */
   *prev = PORT->port_next;

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Bridge] Update to RSTP lib
@ 2012-12-28 13:47 Christopher Merck
  0 siblings, 0 replies; 6+ messages in thread
From: Christopher Merck @ 2012-12-28 13:47 UTC (permalink / raw)
  To: bridge@lists.linux-foundation.org


[-- Attachment #1.1: Type: text/plain, Size: 1124 bytes --]

> > On Mon, Jan 12, 2009 at 1:22 PM, Manish Talreja <mtalreja at crestron.com> wrote:

> > > We have been using the RSTP implementation posted at this location:

> > >

> > > https://lists.linux-foundation.org/pipermail/bridge/2008-May/00585

> > > 9.html

> > >

> > >

> > >

> > > We have modified rstp.c and rstp.h to work with compilers other than gcc.

> > > Attached are two patch files that show our changes. Currently, we

> > > have tested this code with the Diab compiler (v 4.4a) and with IAR

> > > compiler (v 5.11).

> > >



We have further modified rstp.c and rstp.h to work with more compilers. The attached unified diff shows the changes. This has currently been tested with VC++ 2008.

Regards,
 Chris Merck



This e-mail message and all attachments transmitted with it may contain legally privileged and confidential information intended solely for the use of the addressee. If you are not the intended recipient, you are hereby notified that any reading, dissemination, distribution, copying, or other use of this message or its attachments is strictly prohibited.

[-- Attachment #1.2: Type: text/html, Size: 2987 bytes --]

[-- Attachment #2: rstp.patch --]
[-- Type: application/octet-stream, Size: 16333 bytes --]

--- rstp.h.orig	2012-12-21 09:24:35.492806900 -0500
+++ rstp.h	2012-12-21 09:24:45.710741400 -0500
@@ -222,6 +222,7 @@
 void *STP_OUT_mem_zalloc(unsigned int size);
 
 void STP_OUT_mem_free(void *);
+int STP_OUT_topologychanged(int setOrRead);
 
 
 
@@ -232,6 +233,10 @@
 #define COMPILER_INLINE_PACK  __packed__
 #elif defined (__ICCARM__)
 #define COMPILER_INLINE_PACK
+#elif defined WIN32
+#define COMPILER_INLINE_PACK
+#elif defined WINCE
+#define COMPILER_INLINE_PACK
 #else
 DEFINE PACKING FOR YOUR COMPILER
 #endif
--- rstp.c.orig	2012-12-21 09:27:38.374409000 -0500
+++ rstp.c	2012-12-21 09:27:53.053914900 -0500
@@ -28,11 +28,21 @@
   Section references in the code refer to those parts of the standard.
 *********************************************************************/
 
+/* Changes: 
+23 MAY 12: 
+    renamed WINCE_BUILD, STP_DEBUG_PRINT, STP_ERROR_PRINT, and STP_ABORT macros
+    added STP_ERROR_PRINT_NO_REFS macro to avoid dereference of uninitialized pointers (bug in original code)
+*/
+
 #include "rstp.h"
 #include <string.h>
 
-/* Macros to be used in the later parts */
+#ifdef WINCE_BUILD
+    #pragma warning(disable: 4127 4244 4100 4389 4245 4700)
+    #define inline
+#endif
 
+/* Macros to be used in the later parts */
 
 #ifndef NULL
 #define NULL ((void *)0)
@@ -107,19 +117,21 @@
 
 /* Logging macros */
 #ifndef __GNUC__
-#define DEBUG(...) \
+#define STP_DEBUG_PRINT(...) \
   STP_OUT_logmsg(BRIDGE->user_ref, PORT->user_ref, STP_LOG_LEVEL_DEBUG, __VA_ARGS__)
-#define ERROR(...) \
+#define STP_ERROR_PRINT(...) \
   STP_OUT_logmsg(BRIDGE->user_ref, PORT->user_ref, STP_LOG_LEVEL_ERROR, __VA_ARGS__)
-#define ABORT(...) \
-  { ERROR(__VA_ARGS__);}
+#define STP_ERROR_PRINT_NO_REFS(...) \
+  STP_OUT_logmsg(NULL, NULL, STP_LOG_LEVEL_ERROR, __VA_ARGS__)
+#define STP_ABORT(...) \
+  { STP_ERROR_PRINT(__VA_ARGS__);}
 #else
-#define DEBUG(fmt...) \
+#define STP_DEBUG_PRINT(fmt...) \
   STP_OUT_logmsg(BRIDGE->user_ref, PORT->user_ref, STP_LOG_LEVEL_DEBUG, fmt)
-#define ERROR(fmt...) \
+#define STP_ERROR_PRINT(fmt...) \
   STP_OUT_logmsg(BRIDGE->user_ref, PORT->user_ref, STP_LOG_LEVEL_ERROR, fmt)
-#define ABORT(fmt...) \
-  ({ ERROR(fmt); *(int *)0 = 1; })
+#define STP_ABORT(fmt...) \
+  ({ STP_ERROR_PRINT(fmt); *(int *)0 = 1; })
 #endif
 
 /***************** Macros for state machine descriptions ********************/
@@ -151,7 +163,7 @@
 {                                                                           \
   if (_cond) {                                                               \
     new_state = STN(_new_state);                                             \
-    DEBUG("%s: Transition to state %s\n", _STR(STM_NAME), #_new_state);      \
+    STP_DEBUG_PRINT("%s: Transition to state %s\n", _STR(STM_NAME), #_new_state);      \
     goto STM_LABEL(_new_state);                                              \
   }                                                                          \
 }
@@ -160,7 +172,7 @@
 ({                                                                           \
   if (_cond) {                                                               \
     new_state = STN(_new_state);                                             \
-    DEBUG("%s: Transition to state %s\n", _STR(STM_NAME), #_new_state);      \
+    STP_DEBUG_PRINT("%s: Transition to state %s\n", _STR(STM_NAME), #_new_state);      \
     goto STM_LABEL(_new_state);                                              \
   }                                                                          \
 })
@@ -189,13 +201,13 @@
 #define BG(...)                                              \
          case STN(BEGIN):                                                \
              __VA_ARGS__                                                \
-             ABORT("%s: No where to go from BEGIN\n", _STR(STM_NAME));   \
+             STP_ABORT("%s: No where to go from BEGIN\n", _STR(STM_NAME));   \
              return -1
 #else
 #define BG(_transitions...)                                              \
          case STN(BEGIN):                                                \
              _transitions                                                \
-             ABORT("%s: No where to go from BEGIN\n", _STR(STM_NAME));   \
+             STP_ABORT("%s: No where to go from BEGIN\n", _STR(STM_NAME));   \
              return -1
 #endif
 
@@ -221,6 +233,12 @@
 /* This just barely works. _args expansion includes commas */
 //int STM_FUNC(STM_NAME)(int state, int single_step, _args)
 
+#ifdef WINCE_BUILD
+    #define FUNC __FUNCTION__
+#else
+    #define FUNC __func__
+#endif
+
 #ifndef __GNUC__
 #define STM(_args, ...)                                 \
 static STM_FUNC_DECL(STM_FUNC(STM_NAME), _args)                  \
@@ -229,7 +247,7 @@
   switch (state) {                                               \
     __VA_ARGS__                                                  \
       default:                                                   \
-    ABORT("%s: Got unknown state %d\n", __func__, state);        \
+    STP_ABORT("%s: Got unknown state %d\n", FUNC, state);        \
     return -1;                                                   \
   }                                                              \
 }
@@ -241,7 +259,7 @@
   switch (state) {                                               \
     _contents                                                    \
       default:                                                   \
-    ABORT("%s: Got unknown state %d\n", __func__, state);        \
+    STP_ABORT("%s: Got unknown state %d\n", FUNC, state);        \
     return -1;                                                   \
   }                                                              \
 }
@@ -295,32 +313,31 @@
 
 typedef uint bool;
 
-#ifdef __ICCARM__
-#pragma pack(1)
-#endif
+#if defined(__ICCARM__) || defined(WINCE_BUILD)
+#pragma pack(1)
+#endif
 typedef COMPILER_INLINE_PACK struct _BridgeId {
   uchar bridge_identifier_priority[2];
   uchar bridge_address[6];
 } __attribute__((packed)) BridgeId;
-#ifdef __ICCARM__
-#pragma pack()
-#endif
-
-#ifdef __ICCARM__
-#pragma pack(1)
-#endif
+#if defined(__ICCARM__) || defined(WINCE_BUILD)
+#pragma pack()
+#endif
+
+#if defined(__ICCARM__) || defined(WINCE_BUILD)
+#pragma pack(1)
+#endif
 typedef COMPILER_INLINE_PACK struct _PathCost{
   uchar cost[4];
 } __attribute__((packed)) PathCost;
-#ifdef __ICCARM__
-#pragma pack()
-#endif
+#if defined(__ICCARM__) || defined(WINCE_BUILD)
+#pragma pack()
+#endif
 
 static inline uint path_cost_to_uint(PathCost x)
 {
   return
     (x.cost[0] << 24) + (x.cost[1] << 16) + (x.cost[2] << 8) + (x.cost[3]);
-
 }
 
 #ifndef __GNUC__
@@ -355,24 +372,24 @@
 
 #endif
 
-#ifdef __ICCARM__
-#pragma pack(1)
-#endif
+#if defined(__ICCARM__) || defined(WINCE_BUILD)
+#pragma pack(1)
+#endif
 typedef COMPILER_INLINE_PACK struct _PortId {
   uchar port_id[2]; /* 4 high bits of priority and 12 for Id */
 } __attribute__((packed)) PortId;
-#ifdef __ICCARM__
-#pragma pack()
-#endif
+#if defined(__ICCARM__) || defined(WINCE_BUILD)
+#pragma pack()
+#endif
 
 static inline uint port_id_to_uint(PortId p)
 {
   return (p.port_id[0] << 8) + p.port_id[1];
 }
 
-#ifdef __ICCARM__
-#pragma pack(1)
-#endif
+#if defined(__ICCARM__) || defined(WINCE_BUILD)
+#pragma pack(1)
+#endif
 typedef COMPILER_INLINE_PACK struct _PriorityVector
 {
   BridgeId root_bridge_id;
@@ -381,13 +398,13 @@
   PortId   designated_port_id;
   PortId   bridge_port_id;
 } __attribute__((packed)) PriorityVector;
-#ifdef __ICCARM__
-#pragma pack()
-#endif
-
-#ifdef __ICCARM__
-#pragma pack(1)
-#endif
+#if defined(__ICCARM__) || defined(WINCE_BUILD)
+#pragma pack()
+#endif
+
+#if defined(__ICCARM__) || defined(WINCE_BUILD)
+#pragma pack(1)
+#endif
 /* First 4 components of priority vector */
 typedef COMPILER_INLINE_PACK struct _PriorityVector4
 {
@@ -396,9 +413,9 @@
   BridgeId designated_bridge_id;
   PortId   designated_port_id;
 } __attribute__((packed)) PriorityVector4;
-#ifdef __ICCARM__
-#pragma pack()
-#endif
+#if defined(__ICCARM__) || defined(WINCE_BUILD)
+#pragma pack()
+#endif
 
 typedef struct _Times
 {
@@ -467,9 +484,9 @@
 
 #define STP_PROTOCOL_ID 0
 
-#ifdef __ICCARM__
-#pragma pack(1)
-#endif
+#if defined(__ICCARM__) || defined(WINCE_BUILD)
+#pragma pack(1)
+#endif
 typedef COMPILER_INLINE_PACK struct _RawBPDU
 {
   uint16 protocol_id;
@@ -506,9 +523,9 @@
   uchar RST_END[0];
 #endif
 } __attribute__((packed)) RawBPDU;
-#ifdef __ICCARM__
-#pragma pack()
-#endif
+#if defined(__ICCARM__) || defined(WINCE_BUILD)
+#pragma pack()
+#endif
 
 
 #ifndef __GNU_C
@@ -1156,6 +1173,7 @@
     /* Becoming nonzero from zero */
     BRIDGE->tc_count++;
     BRIDGE->time_since_tc = 0;
+    STP_OUT_topologychanged(1);
   }
 
   BRIDGE->tcWhile_count+= change;
@@ -1382,7 +1400,7 @@
 
   set_bpdu_times(&b, &designatedTimes);
 
-  DEBUG("Sending Config BPDU\n");
+  STP_DEBUG_PRINT("Sending Config BPDU\n");
 #ifndef __GNUC__
   STP_OUT_tx_bpdu(PORT->user_ref, &b, CONFIG_END_SIZE_BYTES);
 #else
@@ -1424,7 +1442,7 @@
 
   b.version1_len = 0;
 
-  DEBUG("Sending RST BPDU\n");
+  STP_DEBUG_PRINT("Sending RST BPDU\n");
 #ifndef __GNUC__
   STP_OUT_tx_bpdu(PORT->user_ref, &b, RST_END_SIZE_BYTES);
 #else
@@ -1441,7 +1459,7 @@
   b.version = 0;
   b.type = BPDUTypeTCN;
 
-  DEBUG("Sending TCN BPDU\n");
+  STP_DEBUG_PRINT("Sending TCN BPDU\n");
 #ifndef __GNUC__
   STP_OUT_tx_bpdu(PORT->user_ref, &b, TCN_END_SIZE_BYTES);
 #else
@@ -1594,9 +1612,9 @@
                  }
                  break;
                default:
-                 ERROR("Unknown value for infoIs: %d\n", infoIs);
+                 STP_ERROR_PRINT("Unknown value for infoIs: %d\n", infoIs);
                }
-                 DEBUG("Selected Role Set to: %d\n", selectedRole);
+                 STP_DEBUG_PRINT("Selected Role Set to: %d\n", selectedRole);
                );
 }
 
@@ -2642,7 +2660,7 @@
     return; // No action - else we might fail the next test
 
   if (rcvdBpdu) {
-    ABORT("Port hasn't processed old BPDU\n");
+    STP_ABORT("Port hasn't processed old BPDU\n");
     return;
   }
 
@@ -2652,7 +2670,7 @@
   if (p->protocol_id != STP_PROTOCOL_ID)
     return;
 
-  DEBUG("Received BPDU of type %d\n", p->type);
+  STP_DEBUG_PRINT("Received BPDU of type %d\n", p->type);
 
   if (// 9.3.4 a)
       (p->type == BPDUTypeConfig &&
@@ -2673,7 +2691,7 @@
     /* BPDU has been validated */
   }
   else {
-    DEBUG("BPDU validation failed\n");
+    STP_DEBUG_PRINT("BPDU validation failed\n");
     return;
   }
 
@@ -2723,7 +2741,7 @@
 static int check_times(BRIDGE_ARGS_PROTO, int max_age, int fwd_delay)
 {
   if (2*(fwd_delay - 1) < max_age) {
-    ERROR("Configured BridgeTimes doesn't meet "
+    STP_ERROR_PRINT("Configured BridgeTimes doesn't meet "
           "2 * (Bridge Foward Delay - 1) >= Bridge Max Age\n");
     return -1;
   }
@@ -2731,7 +2749,7 @@
   // This condition never fails, with hello_time == 2 && max_age >= 6.
   // So no need to check.
   //if (max_age < 2*(BridgeHelloTime + 1)) {
-  //  ERROR("Doesn't meet Bridge Max Age >= 2 * (Bridge Hello Time + 1) ");
+  //  STP_ERROR_PRINT("Doesn't meet Bridge Max Age >= 2 * (Bridge Hello Time + 1) ");
   //  return -1;
   //}
 
@@ -2750,7 +2768,7 @@
 
   if (cfg->set_bridge_protocol_version) {
     if (cfg->bridge_protocol_version != 0 && cfg->bridge_protocol_version != 2) {
-      ERROR("Protocol version must be 0 or 2\n");
+      STP_ERROR_PRINT("Protocol version must be 0 or 2\n");
       r = -1;
     }
   }
@@ -2760,7 +2778,7 @@
   if (cfg->set_bridge_priority) {
     // 17.14 - Table 17-2
     if (cfg->bridge_priority & ~0xf000) {
-      ERROR("Bridge Priority restricted to 0-61440 in steps of 4096\n");
+      STP_ERROR_PRINT("Bridge Priority restricted to 0-61440 in steps of 4096\n");
       r = -1;
     }
   }
@@ -2771,7 +2789,7 @@
   if (cfg->set_bridge_hello_time) {
     // 17.14 - Table 17-1
     if (cfg->bridge_hello_time != BridgeHelloTime) {
-      ERROR("Hello Time cannot be changed from 2s in RSTP\n");
+      STP_ERROR_PRINT("Hello Time cannot be changed from 2s in RSTP\n");
       r = -1;
     }
     new_times.hello_time = cfg->bridge_hello_time;
@@ -2781,7 +2799,7 @@
   if (cfg->set_bridge_max_age) {
     // 17.14 - Table 17-1
     if (cfg->bridge_max_age < 6 || cfg->bridge_max_age > 40) {
-      ERROR("Bridge Max Age must be between 6 and 40\n");
+      STP_ERROR_PRINT("Bridge Max Age must be between 6 and 40\n");
       r = -1;
     }
     new_times.max_age = cfg->bridge_max_age;
@@ -2791,7 +2809,7 @@
   if (cfg->set_bridge_forward_delay) {
     // 17.14 - Table 17-1
     if (cfg->bridge_forward_delay < 4 || cfg->bridge_forward_delay > 30) {
-      ERROR("Bridge Forward Delay must be between 4 and 30\n");
+      STP_ERROR_PRINT("Bridge Forward Delay must be between 4 and 30\n");
       r = -1;
     }
     new_times.forward_delay = cfg->bridge_forward_delay;
@@ -2804,7 +2822,7 @@
   if (cfg->set_bridge_tx_hold_count) {
     // 17.14 - Table 17-1
     if (cfg->bridge_tx_hold_count < 1 || cfg->bridge_tx_hold_count > 10) {
-      ERROR("Transmit Hold Count must be between 1 and 10\n");
+      STP_ERROR_PRINT("Transmit Hold Count must be between 1 and 10\n");
       r = -1;
     }
   }
@@ -2906,7 +2924,7 @@
   if (cfg->set_port_priority) {
     // 17.14 - Table 17-2
     if (cfg->port_priority & ~0xf0) {
-      ERROR("Port Priority restricted to 0-240 in steps of 16\n");
+      STP_ERROR_PRINT("Port Priority restricted to 0-240 in steps of 16\n");
       r = -1;
     }
   }
@@ -2914,28 +2932,28 @@
   if (cfg->set_port_pathcost) {
     // 17.14 - Note 3 and Table 17-3
     if (cfg->port_pathcost > 200000000) {
-      ERROR("Port path cost restricted to 1-200,000,000 or 0 for auto\n");
+      STP_ERROR_PRINT("Port path cost restricted to 1-200,000,000 or 0 for auto\n");
       r = -1;
     }
   }
 
   if (cfg->set_port_admin_edge) {
     if (cfg->port_admin_edge != 0 && cfg->port_admin_edge != 1) {
-      ERROR("Admin Edge must be 0 or 1\n");
+      STP_ERROR_PRINT("Admin Edge must be 0 or 1\n");
       r = -1;
     }
   }
 
   if (cfg->set_port_auto_edge) {
     if (cfg->port_auto_edge != 0 && cfg->port_auto_edge != 1) {
-      ERROR("Auto Edge must be 0 or 1\n");
+      STP_ERROR_PRINT("Auto Edge must be 0 or 1\n");
       r = -1;
     }
   }
 
   if (cfg->set_port_admin_p2p) {
     if (cfg->port_admin_p2p < 0 || cfg->port_admin_p2p > 2) {
-      ERROR("Admin P2P must be "
+      STP_ERROR_PRINT("Admin P2P must be "
             "0 (force false), 1 (force true), or 2 (auto)\n");
       r = -1;
     }
@@ -3126,7 +3144,7 @@
   Bridge *BRIDGE = PORT->bridge;
 
   if (!BRIDGE->stp_on) {
-    ERROR("Bridge is down\n");
+    STP_ERROR_PRINT("Bridge is down\n");
     return -1;
   }
 
@@ -3221,7 +3239,7 @@
   Bridge *b = STP_OUT_mem_zalloc(sizeof(Bridge));
 
   if (!b) {
-    ERROR("Couldn't allocate memory for bridge\n");
+    STP_ERROR_PRINT_NO_REFS("Couldn't allocate memory for bridge\n");
     return NULL;
   }
 
@@ -3268,14 +3286,14 @@
   Port *PORT;
 
   if (port_number == 0 || (port_number & ~0x0fff)) {
-    ERROR("Port id should be in the range 1 - 1023\n");
+    STP_ERROR_PRINT_NO_REFS("Port id should be in the range 1 - 1023\n");
     return NULL;
   }
 
   p = STP_OUT_mem_zalloc(sizeof(Port));
 
   if (!p) {
-    ERROR("Couldn't allocate memory for port\n");
+    STP_ERROR_PRINT_NO_REFS("Couldn't allocate memory for port\n");
     return NULL;
   }
 
@@ -3312,7 +3330,7 @@
 void STP_IN_port_delete(Port *PORT)
 {
   Bridge *BRIDGE = PORT->bridge;
-  Port **prev; 
+  Port **prev;
 
   /* Disable */
   if (portEnabled) {
@@ -3326,7 +3344,7 @@
     prev = &(*prev)->port_next;
 
   if (*prev == NULL)
-    ABORT("port not in its bridge's list\n");
+    STP_ABORT("port not in its bridge's list\n");
 
   /* Remove from list */
   *prev = PORT->port_next;

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-12-28 13:47 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-01-12 19:22 [Bridge] Update to RSTP lib Manish Talreja
2009-01-12 20:00 ` richardvoigt
2009-01-12 20:42   ` Manish Talreja
2009-01-12 21:32     ` Stephen Hemminger
  -- strict thread matches above, loose matches on Subject: below --
2012-12-21 16:31 Christopher Merck
2012-12-28 13:47 Christopher Merck

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox