Project

General

Profile

Actions
Copy link

Bug #859

closed
MS

1.4.3: src/flow-hash.c has anonymous structs

Bug #859: 1.4.3: src/flow-hash.c has anonymous structs

Added by Mark Solaris about 13 years ago. Updated almost 9 years ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
-
Affected Versions:
Effort:
Difficulty:
Label:

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;

VJ Updated by Victor Julien about 13 years ago Actions
Copy link
 #2

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.

MS Updated by Mark Solaris about 13 years ago Actions
Copy link
 #3

How is the github thing done?

MS Updated by Mark Solaris about 13 years ago Actions
Copy link
 #5

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.

VJ Updated by Victor Julien almost 13 years ago Actions
Copy link
 #6

  • Target version set to TBD

AH Updated by Andreas Herz over 10 years ago Actions
Copy link
 #7

  • Status changed from New to Closed

VJ Updated by Victor Julien almost 9 years ago Actions
Copy link
 #8

  • Target version deleted (TBD)
Actions
Copy link

Also available in: PDF Atom