Bug #859
closed1.4.3: src/flow-hash.c has anonymous structs
Description
Solaris Sun Studio 12.3 compiler is picking up dodgy coding:
src/flow-hash.c: "flow-hash.c", line 170: warning: anonymous struct declaration "flow-hash.c", line 169: warning: anonymous union declaration "flow-hash.c", line 182: warning: anonymous struct declaration "flow-hash.c", line 181: warning: anonymous union declaration "flow-hash.c", line 212: improper member use: src "flow-hash.c", line 212: assignment type mismatch: array[4] of unsigned int "=" unsigned int "flow-hash.c", line 213: improper member use: dst "flow-hash.c", line 213: assignment type mismatch: array[4] of unsigned int "=" unsigned int "flow-hash.c", line 215: improper member use: src "flow-hash.c", line 215: assignment type mismatch: array[4] of unsigned int "=" unsigned int "flow-hash.c", line 216: improper member use: dst "flow-hash.c", line 216: assignment type mismatch: array[4] of unsigned int "=" unsigned int "flow-hash.c", line 219: improper member use: sp "flow-hash.c", line 220: improper member use: dp "flow-hash.c", line 222: improper member use: sp "flow-hash.c", line 223: improper member use: dp "flow-hash.c", line 225: improper member use: proto "flow-hash.c", line 226: improper member use: recur "flow-hash.c", line 236: improper member use: src "flow-hash.c", line 236: assignment type mismatch: array[4] of unsigned int "=" unsigned int "flow-hash.c", line 237: improper member use: dst "flow-hash.c", line 237: assignment type mismatch: array[4] of unsigned int "=" unsigned int "flow-hash.c", line 239: improper member use: src "flow-hash.c", line 239: assignment type mismatch: array[4] of unsigned int "=" unsigned int "flow-hash.c", line 240: improper member use: dst "flow-hash.c", line 240: assignment type mismatch: array[4] of unsigned int "=" unsigned int "flow-hash.c", line 243: improper member use: sp "flow-hash.c", line 244: improper member use: dp "flow-hash.c", line 246: improper member use: sp [...]
I gave the structs useful names and made the variables more descriptive. It now passes compilation. Diff is here:
--- suricata-1.4.3/src/flow-hash.c.orig Mon Jul 8 11:32:13 2013 +++ suricata-1.4.3/src/flow-hash.c Mon Jul 8 11:38:21 2013 @@ -168,12 +168,12 @@ typedef struct FlowHashKey4_ { union { struct { - uint32_t src, dst; - uint16_t sp, dp; - uint16_t proto; /**< u16 so proto and recur add up to u32 */ - uint16_t recur; /**< u16 so proto and recur add up to u32 */ - }; - uint32_t u32[4]; + uint32_t fhk4src, fhk4dst; + uint16_t fhk4sp, fhk4dp; + uint16_t fhk4proto; /**< u16 so proto and recur add up to u32 */ + uint16_t fhk4recur; /**< u16 so proto and recur add up to u32 */ + } fhk4; + uint32_t fhk4u32[4]; }; } FlowHashKey4; @@ -180,12 +180,12 @@ typedef struct FlowHashKey6_ { union { struct { - uint32_t src[4], dst[4]; - uint16_t sp, dp; - uint16_t proto; /**< u16 so proto and recur add up to u32 */ - uint16_t recur; /**< u16 so proto and recur add up to u32 */ - }; - uint32_t u32[10]; + uint32_t fhk6src[4], fhk6dst[4]; + uint16_t fhk6sp, fhk6dp; + uint16_t fhk6proto; /**< u16 so proto and recur add up to u32 */ + uint16_t fhk6recur; /**< u16 so proto and recur add up to u32 */ + } fhk6; + uint32_t fhk6u32[10]; }; } FlowHashKey6; @@ -209,23 +209,23 @@ if (p->tcph != NULL || p->udph != NULL) { FlowHashKey4 fhk; if (p->src.addr_data32[0] > p->dst.addr_data32[0]) { - fhk.src = p->src.addr_data32[0]; - fhk.dst = p->dst.addr_data32[0]; + fhk.fhk4.fhk4src = p->src.addr_data32[0]; + fhk.fhk4.fhk4dst = p->dst.addr_data32[0]; } else { - fhk.src = p->dst.addr_data32[0]; - fhk.dst = p->src.addr_data32[0]; + fhk.fhk4.fhk4src = p->dst.addr_data32[0]; + fhk.fhk4.fhk4dst = p->src.addr_data32[0]; } if (p->sp > p->dp) { - fhk.sp = p->sp; - fhk.dp = p->dp; + fhk.fhk4.fhk4sp = p->sp; + fhk.fhk4.fhk4dp = p->dp; } else { - fhk.sp = p->dp; - fhk.dp = p->sp; + fhk.fhk4.fhk4sp = p->dp; + fhk.fhk4.fhk4dp = p->sp; } - fhk.proto = (uint16_t)p->proto; - fhk.recur = (uint16_t)p->recursion_level; + fhk.fhk4.fhk4proto = (uint16_t)p->proto; + fhk.fhk4.fhk4recur = (uint16_t)p->recursion_level; - uint32_t hash = hashword(fhk.u32, 4, flow_config.hash_rand); + uint32_t hash = hashword(fhk.fhk4u32, 4, flow_config.hash_rand); key = hash % flow_config.hash_size; } else if (ICMPV4_DEST_UNREACH_IS_VALID(p)) { @@ -233,74 +233,74 @@ uint32_t pdst = IPV4_GET_RAW_IPDST_U32(ICMPV4_GET_EMB_IPV4(p)); FlowHashKey4 fhk; if (psrc > pdst) { - fhk.src = psrc; - fhk.dst = pdst; + fhk.fhk4.fhk4src = psrc; + fhk.fhk4.fhk4dst = pdst; } else { - fhk.src = pdst; - fhk.dst = psrc; + fhk.fhk4.fhk4src = pdst; + fhk.fhk4.fhk4dst = psrc; } if (p->icmpv4vars.emb_sport > p->icmpv4vars.emb_dport) { - fhk.sp = p->icmpv4vars.emb_sport; - fhk.dp = p->icmpv4vars.emb_dport; + fhk.fhk4.fhk4sp = p->icmpv4vars.emb_sport; + fhk.fhk4.fhk4dp = p->icmpv4vars.emb_dport; } else { - fhk.sp = p->icmpv4vars.emb_dport; - fhk.dp = p->icmpv4vars.emb_sport; + fhk.fhk4.fhk4sp = p->icmpv4vars.emb_dport; + fhk.fhk4.fhk4dp = p->icmpv4vars.emb_sport; } - fhk.proto = (uint16_t)ICMPV4_GET_EMB_PROTO(p); - fhk.recur = (uint16_t)p->recursion_level; + fhk.fhk4.fhk4proto = (uint16_t)ICMPV4_GET_EMB_PROTO(p); + fhk.fhk4.fhk4recur = (uint16_t)p->recursion_level; - uint32_t hash = hashword(fhk.u32, 4, flow_config.hash_rand); + uint32_t hash = hashword(fhk.fhk4u32, 4, flow_config.hash_rand); key = hash % flow_config.hash_size; } else { FlowHashKey4 fhk; if (p->src.addr_data32[0] > p->dst.addr_data32[0]) { - fhk.src = p->src.addr_data32[0]; - fhk.dst = p->dst.addr_data32[0]; + fhk.fhk4.fhk4src = p->src.addr_data32[0]; + fhk.fhk4.fhk4dst = p->dst.addr_data32[0]; } else { - fhk.src = p->dst.addr_data32[0]; - fhk.dst = p->src.addr_data32[0]; + fhk.fhk4.fhk4src = p->dst.addr_data32[0]; + fhk.fhk4.fhk4dst = p->src.addr_data32[0]; } - fhk.sp = 0xfeed; - fhk.dp = 0xbeef; - fhk.proto = (uint16_t)p->proto; - fhk.recur = (uint16_t)p->recursion_level; + fhk.fhk4.fhk4sp = 0xfeed; + fhk.fhk4.fhk4dp = 0xbeef; + fhk.fhk4.fhk4proto = (uint16_t)p->proto; + fhk.fhk4.fhk4recur = (uint16_t)p->recursion_level; - uint32_t hash = hashword(fhk.u32, 4, flow_config.hash_rand); + uint32_t hash = hashword(fhk.fhk4u32, 4, flow_config.hash_rand); key = hash % flow_config.hash_size; } } else if (p->ip6h != NULL) { FlowHashKey6 fhk; if (FlowHashRawAddressIPv6GtU32(p->src.addr_data32, p->dst.addr_data32)) { - fhk.src[0] = p->src.addr_data32[0]; - fhk.src[1] = p->src.addr_data32[1]; - fhk.src[2] = p->src.addr_data32[2]; - fhk.src[3] = p->src.addr_data32[3]; - fhk.dst[0] = p->dst.addr_data32[0]; - fhk.dst[1] = p->dst.addr_data32[1]; - fhk.dst[2] = p->dst.addr_data32[2]; - fhk.dst[3] = p->dst.addr_data32[3]; + fhk.fhk6.fhk6src[0] = p->src.addr_data32[0]; + fhk.fhk6.fhk6src[1] = p->src.addr_data32[1]; + fhk.fhk6.fhk6src[2] = p->src.addr_data32[2]; + fhk.fhk6.fhk6src[3] = p->src.addr_data32[3]; + fhk.fhk6.fhk6dst[0] = p->dst.addr_data32[0]; + fhk.fhk6.fhk6dst[1] = p->dst.addr_data32[1]; + fhk.fhk6.fhk6dst[2] = p->dst.addr_data32[2]; + fhk.fhk6.fhk6dst[3] = p->dst.addr_data32[3]; } else { - fhk.src[0] = p->dst.addr_data32[0]; - fhk.src[1] = p->dst.addr_data32[1]; - fhk.src[2] = p->dst.addr_data32[2]; - fhk.src[3] = p->dst.addr_data32[3]; - fhk.dst[0] = p->src.addr_data32[0]; - fhk.dst[1] = p->src.addr_data32[1]; - fhk.dst[2] = p->src.addr_data32[2]; - fhk.dst[3] = p->src.addr_data32[3]; + fhk.fhk6.fhk6src[0] = p->dst.addr_data32[0]; + fhk.fhk6.fhk6src[1] = p->dst.addr_data32[1]; + fhk.fhk6.fhk6src[2] = p->dst.addr_data32[2]; + fhk.fhk6.fhk6src[3] = p->dst.addr_data32[3]; + fhk.fhk6.fhk6dst[0] = p->src.addr_data32[0]; + fhk.fhk6.fhk6dst[1] = p->src.addr_data32[1]; + fhk.fhk6.fhk6dst[2] = p->src.addr_data32[2]; + fhk.fhk6.fhk6dst[3] = p->src.addr_data32[3]; } if (p->sp > p->dp) { - fhk.sp = p->sp; - fhk.dp = p->dp; + fhk.fhk6.fhk6sp = p->sp; + fhk.fhk6.fhk6dp = p->dp; } else { - fhk.sp = p->dp; - fhk.dp = p->sp; + fhk.fhk6.fhk6sp = p->dp; + fhk.fhk6.fhk6dp = p->sp; } - fhk.proto = (uint16_t)p->proto; - fhk.recur = (uint16_t)p->recursion_level; + fhk.fhk6.fhk6proto = (uint16_t)p->proto; + fhk.fhk6.fhk6recur = (uint16_t)p->recursion_level; - uint32_t hash = hashword(fhk.u32, 10, flow_config.hash_rand); + uint32_t hash = hashword(fhk.fhk6u32, 10, flow_config.hash_rand); key = hash % flow_config.hash_size; } else key = 0;
Updated by Mark Solaris almost 11 years ago
This thread has more on anonymous structs in unions.
http://stackoverflow.com/questions/12653500/how-to-access-a-struct-member-inside-a-union-in-c
Updated by Victor Julien almost 11 years ago
Thanks for looking into getting things working on Solaris. Would like to suggest a better way to report and fix though as the ticket system isn't the best way in this case. I think it would be best to submit these patches in a github pull request. In case of non-patched issues and questions it may be best to discuss on our oisf-devel list. Thanks.
Updated by Mark Solaris almost 11 years ago
I'll do that eventually, it's mid-flu season here so my motivation to deeply engage in Yet Another Project is low.. especially when I can get it into the system and search engine indexable by using this redmine stuff. =) Sorry! I'm under the gun to get this out the door.
There's six more issues to be created for code fixes, and one with the environmental stuff for a Solaris build. The build has passed 'make install-full' so I'll be using it in anger soon.