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 over 12 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 over 12 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 over 12 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.