bzr commit into mysql-5.5-bugfixing branch (marc.alff:3203) Bug#56522

[Marc Alff]

#At file:///home/malff/BZR_TREE/mysql-5.5-bugfixing-56522/ based on revid:wlad@stripped

 3203 Marc Alff	2010-09-02
      Bug#56522 Compiler warnings in mysys/lf_hash.c
      
      Before this fix, the file mysys/lf_hash.c would build with compiler
      warnings, such as "dereferencing type-punned pointer might break
      strict-aliasing rules".
      
      For GCC in particular, gcc 4.3.2 would generate warnings
      when compiled with -Wstrict-aliasing=2,
      and no warnings with -Wstrict-aliasing=3
      
      This fix simplifies the code to avoid complex constructs,
      and clarifies type casting.
      
      With the fix, the build is now clean for both
      -Wstrict-aliasing=2
      -Wstrict-aliasing=3
      
      While all warnings for all compilers / platforms / compiling options
      may not be fixed, this fix already improves the situation,
      as well as the readability of the code.

    modified:
      mysys/lf_hash.c
=== modified file 'mysys/lf_hash.c'
--- a/mysys/lf_hash.c	2010-07-23 20:13:36 +0000
+++ b/mysys/lf_hash.c	2010-09-03 00:00:36 +0000
@@ -79,7 +79,7 @@ static int lfind(LF_SLIST * volatile *he
   intptr       link;
 
 retry:
-  cursor->prev= (intptr *)head;
+  cursor->prev= (intptr volatile *)head;
   do { /* PTR() isn't necessary below, head is a dummy node */
     cursor->curr= (LF_SLIST *)(*cursor->prev);
     _lf_pin(pins, 1, cursor->curr);
@@ -121,8 +121,22 @@ retry:
         we found a deleted node - be nice, help the other thread
         and remove this deleted node
       */
-      if (my_atomic_casptr((void **)cursor->prev,
-                           (void **)&cursor->curr, cursor->next))
+      intptr volatile * typed_hdl_1;
+      void * volatile * opaque_hdl_1;
+      LF_SLIST ** typed_hdl_2;
+      void ** opaque_hdl_2;
+      /*
+        To avoid compiler warnings such as
+        'dereferencing type-punned pointer will break strict-aliasing rules',
+        when compiling with gcc and -Wstrict-aliasing=1 2 or 3,
+        the casts and pointer dereferencing operations are done
+        in very explicit steps.
+      */
+      typed_hdl_1= cursor->prev;
+      opaque_hdl_1= (void * volatile *) typed_hdl_1;
+      typed_hdl_2= &cursor->curr;
+      opaque_hdl_2= (void **) typed_hdl_2;
+      if (my_atomic_casptr(opaque_hdl_1, opaque_hdl_2, cursor->next))
         _lf_alloc_free(pins, cursor->curr);
       else
       {
@@ -165,10 +179,18 @@ static LF_SLIST *linsert(LF_SLIST * vola
     }
     else
     {
+      intptr volatile * typed_hdl_1;
+      void * volatile * opaque_hdl_1;
+      LF_SLIST ** typed_hdl_2;
+      void ** opaque_hdl_2;
       node->link= (intptr)cursor.curr;
       DBUG_ASSERT(node->link != (intptr)node); /* no circular references */
       DBUG_ASSERT(cursor.prev != &node->link); /* no circular references */
-      if (my_atomic_casptr((void **)cursor.prev, (void **)&cursor.curr, node))
+      typed_hdl_1= cursor.prev;
+      opaque_hdl_1= (void * volatile *) typed_hdl_1;
+      typed_hdl_2= &cursor.curr;
+      opaque_hdl_2= (void **) typed_hdl_2;
+      if (my_atomic_casptr(opaque_hdl_1, opaque_hdl_2, node))
       {
         res= 1; /* inserted ok */
         break;
@@ -214,14 +236,28 @@ static int ldelete(LF_SLIST * volatile *
     }
     else
     {
+      intptr volatile * typed_hdl_1;
+      void * volatile * opaque_hdl_1;
+      LF_SLIST ** typed_hdl_2;
+      void ** opaque_hdl_2;
+      intptr typed_ptr_3;
+      void *opaque_ptr_3;
+
+      typed_hdl_1= &(cursor.curr->link);
+      opaque_hdl_1= (void * volatile *) typed_hdl_1;
+      typed_hdl_2= &cursor.next;
+      opaque_hdl_2= (void **) typed_hdl_2;
+      typed_ptr_3= ((intptr)cursor.next) | 1; /* deleted flag */
+      opaque_ptr_3= (void *) typed_ptr_3;
       /* mark the node deleted */
-      if (my_atomic_casptr((void **)&(cursor.curr->link),
-                           (void **)&cursor.next,
-                           (void *)(((intptr)cursor.next) | 1)))
+      if (my_atomic_casptr(opaque_hdl_1, opaque_hdl_2, opaque_ptr_3))
       {
+        typed_hdl_1= cursor.prev;
+        opaque_hdl_1= (void * volatile *) typed_hdl_1;
+        typed_hdl_2= &cursor.curr;
+        opaque_hdl_2= (void **) typed_hdl_2;
         /* and remove it from the list */
-        if (my_atomic_casptr((void **)cursor.prev,
-                             (void **)&cursor.curr, cursor.next))
+        if (my_atomic_casptr(opaque_hdl_1, opaque_hdl_2, cursor.next))
           _lf_alloc_free(pins, cursor.curr);
         else
         {
@@ -473,9 +509,12 @@ static const uchar *dummy_key= (uchar*)"
 static int initialize_bucket(LF_HASH *hash, LF_SLIST * volatile *node,
                               uint bucket, LF_PINS *pins)
 {
+  LF_SLIST * volatile * typed_hdl_1;
+  void * volatile * opaque_hdl_1;
   uint parent= my_clear_highest_bit(bucket);
   LF_SLIST *dummy= (LF_SLIST *)my_malloc(sizeof(LF_SLIST), MYF(MY_WME));
-  LF_SLIST **tmp= 0, *cur;
+  void *tmp= NULL;
+  LF_SLIST *cur;
   LF_SLIST * volatile *el= _lf_dynarray_lvalue(&hash->array, parent);
   if (unlikely(!el || !dummy))
     return -1;
@@ -490,7 +529,9 @@ static int initialize_bucket(LF_HASH *ha
     my_free(dummy);
     dummy= cur;
   }
-  my_atomic_casptr((void **)node, (void **)&tmp, dummy);
+  typed_hdl_1= node;
+  opaque_hdl_1= (void * volatile *) typed_hdl_1;
+  my_atomic_casptr(opaque_hdl_1, &tmp, dummy);
   /*
     note that if the CAS above failed (after linsert() succeeded),
     it would mean that some other thread has executed linsert() for


Attachment: [text/bzr-bundle] bzr/[e-mail]