Eliminate 'label not used' cppcheck false warnings in GC_mark_X

(code refactoring of PUSH_CONTENTS[_HDR] to eliminate 'goto' statement)

* include/private/gc_hdrs.h (HC_GET_HDR): Remove exit_label argument;
replace goto with break; remove do-while(0) (as break is now used to
pass control to some place of the caller).
* include/private/gc_pmark.h (SET_MARK_BIT_EXIT_IF_SET): Likewise.
* include/private/gc_pmark.h [!USE_MARK_BYTES] (OR_WORD_EXIT_IF_SET):
Likewise.
* include/private/gc_pmark.h (PUSH_CONTENTS, PUSH_CONTENTS_HDR):
Remove exit_label argument (and the comment about it).
* include/private/gc_pmark.h (SET_MARK_BIT_EXIT_IF_SET): Refine comment
about the exit.
* mark.c (GC_mark_from): Remove PUSH_CONTENTS exit<N> argument.
* typd_mlc.c (GC_typed_mark_proc): Likewise.
* mark.c (GC_mark_and_push, GC_mark_and_push_stack): Remove
PUSH_CONTENTS_HDR label argument and the label itself.

diff --git a/include/private/gc_hdrs.h b/include/private/gc_hdrs.h
index 4540ac6b038056704c06d02da758807e402fbffb..f9d4ba8a4ef5f511b9ecef07205d98abbbd042e2 100644 (file)
--- a/include/private/gc_hdrs.h
+++ b/include/private/gc_hdrs.h
@@ -103,17 +103,17 @@ typedef struct hce {
 /* is set.                                                              */
 /* Returns zero if p points to somewhere other than the first page      */
 /* of an object, and it is not a valid pointer to the object.           */
-#define HC_GET_HDR(p, hhdr, source, exit_label) \
-        do { \
+#define HC_GET_HDR(p, hhdr, source) \
+        { /* cannot use do-while(0) here */ \
           hdr_cache_entry * hce = HCE(p); \
           if (EXPECT(HCE_VALID_FOR(hce, p), TRUE)) { \
             HC_HIT(); \
             hhdr = hce -> hce_hdr; \
           } else { \
             hhdr = HEADER_CACHE_MISS(p, hce, source); \
-            if (0 == hhdr) goto exit_label; \
+            if (NULL == hhdr) break; /* go to the enclosing loop end */ \
           } \
-        } while (0)
+        }
 
 typedef struct bi {
     hdr * index[BOTTOM_SZ];

diff --git a/include/private/gc_pmark.h b/include/private/gc_pmark.h
index cab2a30a338259341c786b1e2906c623abdffd65..282672af6efc51e2798a5da5a8be21b32c2bedde 100644 (file)
--- a/include/private/gc_pmark.h
+++ b/include/private/gc_pmark.h
@@ -130,57 +130,54 @@ GC_INNER mse * GC_signal_mark_stack_overflow(mse *msp);
 
 /* Push the contents of current onto the mark stack if it is a valid    */
 /* ptr to a currently unmarked object.  Mark it.                        */
-/* If we assumed a standard-conforming compiler, we could probably      */
-/* generate the exit_label transparently.                               */
-#define PUSH_CONTENTS(current, mark_stack_top, mark_stack_limit, \
-                      source, exit_label) \
+#define PUSH_CONTENTS(current, mark_stack_top, mark_stack_limit, source) \
   do { \
     hdr * my_hhdr; \
-    HC_GET_HDR(current, my_hhdr, source, exit_label); \
+    HC_GET_HDR(current, my_hhdr, source); /* contains "break" */ \
     PUSH_CONTENTS_HDR(current, mark_stack_top, mark_stack_limit, \
-                  source, exit_label, my_hhdr, TRUE); \
-  exit_label: ; \
+                      source, my_hhdr, TRUE); \
   } while (0)
 
-/* Set mark bit, exit if it was already set.    */
+/* Set mark bit, exit (using "break" statement) if it is already set.   */
 #ifdef USE_MARK_BYTES
   /* There is a race here, and we may set                               */
   /* the bit twice in the concurrent case.  This can result in the      */
   /* object being pushed twice.  But that's only a performance issue.   */
-# define SET_MARK_BIT_EXIT_IF_SET(hhdr,bit_no,exit_label) \
-    do { \
+# define SET_MARK_BIT_EXIT_IF_SET(hhdr, bit_no) \
+    { /* cannot use do-while(0) here */ \
         char * mark_byte_addr = (char *)hhdr -> hb_marks + (bit_no); \
-        if (*mark_byte_addr) goto exit_label; \
+        if (*mark_byte_addr != 0) break; /* go to the enclosing loop end */ \
         *mark_byte_addr = 1; \
-    } while (0)
+    }
 #else
 # ifdef PARALLEL_MARK
     /* This is used only if we explicitly set USE_MARK_BITS.            */
     /* The following may fail to exit even if the bit was already set.  */
     /* For our uses, that's benign:                                     */
-#   define OR_WORD_EXIT_IF_SET(addr, bits, exit_label) \
-        do { \
+#   define OR_WORD_EXIT_IF_SET(addr, bits) \
+        { /* cannot use do-while(0) here */ \
           if (!(*(addr) & (bits))) { \
             AO_or((volatile AO_t *)(addr), (AO_t)(bits)); \
           } else { \
-            goto exit_label; \
+            break; /* go to the enclosing loop end */ \
           } \
-        } while (0)
+        }
 # else
-#   define OR_WORD_EXIT_IF_SET(addr, bits, exit_label) \
-        do { \
+#   define OR_WORD_EXIT_IF_SET(addr, bits) \
+        { /* cannot use do-while(0) here */ \
            word old = *(addr); \
            word my_bits = (bits); \
-           if (old & my_bits) goto exit_label; \
-           *(addr) = (old | my_bits); \
-        } while (0)
+           if ((old & my_bits) != 0) \
+             break; /* go to the enclosing loop end */ \
+           *(addr) = old | my_bits; \
+        }
 # endif /* !PARALLEL_MARK */
-# define SET_MARK_BIT_EXIT_IF_SET(hhdr,bit_no,exit_label) \
-    do { \
+# define SET_MARK_BIT_EXIT_IF_SET(hhdr, bit_no) \
+    { /* cannot use do-while(0) here */ \
         word * mark_word_addr = hhdr -> hb_marks + divWORDSZ(bit_no); \
-        OR_WORD_EXIT_IF_SET(mark_word_addr, (word)1 << modWORDSZ(bit_no), \
-                            exit_label); \
-    } while (0)
+        OR_WORD_EXIT_IF_SET(mark_word_addr, \
+                (word)1 << modWORDSZ(bit_no)); /* contains "break" */ \
+    }
 #endif /* !USE_MARK_BYTES */
 
 #ifdef PARALLEL_MARK
@@ -228,7 +225,7 @@ GC_INNER mse * GC_signal_mark_stack_overflow(mse *msp);
 /* interior of a large object.                                          */
 #ifdef MARK_BIT_PER_GRANULE
 # define PUSH_CONTENTS_HDR(current, mark_stack_top, mark_stack_limit, \
-                           source, exit_label, hhdr, do_offset_check) \
+                           source, hhdr, do_offset_check) \
   do { \
     size_t displ = HBLKDISPL(current); /* Displacement in block; in bytes. */\
     /* displ is always within range.  If current doesn't point to       */ \
@@ -252,7 +249,7 @@ GC_INNER mse * GC_signal_mark_stack_overflow(mse *msp);
           } else { \
             if (do_offset_check && !GC_valid_offsets[obj_displ]) { \
               GC_ADD_TO_BLACK_LIST_NORMAL(current, source); \
-              goto exit_label; \
+              break; /* go to the end of PUSH_CONTENTS_HDR */ \
             } \
           } \
           gran_displ = 0; \
@@ -264,7 +261,7 @@ GC_INNER mse * GC_signal_mark_stack_overflow(mse *msp);
                              + byte_offset; \
           if (do_offset_check && !GC_valid_offsets[obj_displ]) { \
             GC_ADD_TO_BLACK_LIST_NORMAL(current, source); \
-            goto exit_label; \
+            break; \
           } \
           gran_displ -= gran_offset; \
           base -= obj_displ; \
@@ -274,7 +271,7 @@ GC_INNER mse * GC_signal_mark_stack_overflow(mse *msp);
     GC_ASSERT(gran_displ % BYTES_TO_GRANULES(hhdr -> hb_sz) == 0); \
     TRACE(source, GC_log_printf("GC #%u: passed validity tests\n", \
                                 (unsigned)GC_gc_no)); \
-    SET_MARK_BIT_EXIT_IF_SET(hhdr, gran_displ, exit_label); \
+    SET_MARK_BIT_EXIT_IF_SET(hhdr, gran_displ); \
     TRACE(source, GC_log_printf("GC #%u: previously unmarked\n", \
                                 (unsigned)GC_gc_no)); \
     TRACE_TARGET(base, \
@@ -288,7 +285,7 @@ GC_INNER mse * GC_signal_mark_stack_overflow(mse *msp);
 
 #ifdef MARK_BIT_PER_OBJ
 # define PUSH_CONTENTS_HDR(current, mark_stack_top, mark_stack_limit, \
-                           source, exit_label, hhdr, do_offset_check) \
+                           source, hhdr, do_offset_check) \
   do { \
     size_t displ = HBLKDISPL(current); /* Displacement in block; in bytes. */\
     unsigned32 low_prod, high_prod; \
@@ -309,7 +306,7 @@ GC_INNER mse * GC_signal_mark_stack_overflow(mse *msp);
           } else { \
             if (do_offset_check && !GC_valid_offsets[obj_displ]) { \
               GC_ADD_TO_BLACK_LIST_NORMAL(current, source); \
-              goto exit_label; \
+              break; /* go to the end of PUSH_CONTENTS_HDR */ \
             } \
           } \
           GC_ASSERT(hhdr -> hb_sz > HBLKSIZE || \
@@ -321,7 +318,7 @@ GC_INNER mse * GC_signal_mark_stack_overflow(mse *msp);
           size_t obj_displ = (((low_prod >> 16) + 1) * (hhdr->hb_sz)) >> 16; \
           if (do_offset_check && !GC_valid_offsets[obj_displ]) { \
             GC_ADD_TO_BLACK_LIST_NORMAL(current, source); \
-            goto exit_label; \
+            break; \
           } \
           base -= obj_displ; \
         } \
@@ -331,7 +328,7 @@ GC_INNER mse * GC_signal_mark_stack_overflow(mse *msp);
     GC_ASSERT(high_prod <= HBLK_OBJS(hhdr -> hb_sz)); \
     TRACE(source, GC_log_printf("GC #%u: passed validity tests\n", \
                                 (unsigned)GC_gc_no)); \
-    SET_MARK_BIT_EXIT_IF_SET(hhdr, high_prod, exit_label); \
+    SET_MARK_BIT_EXIT_IF_SET(hhdr, high_prod); \
     TRACE(source, GC_log_printf("GC #%u: previously unmarked\n", \
                                 (unsigned)GC_gc_no)); \
     TRACE_TARGET(base, \

diff --git a/mark.c b/mark.c
index f3dbda923c3d50dd0956cc7892b1878656c09bc1..9b1d0fedfb04c66ee0889b8bb4b75ddcdb72e2e0 100644 (file)
--- a/mark.c
+++ b/mark.c
@@ -747,7 +747,7 @@ GC_INNER mse * GC_mark_from(mse *mark_stack_top, mse *mark_stack,
                   }
 #               endif /* ENABLE_TRACE */
                 PUSH_CONTENTS((ptr_t)current, mark_stack_top,
-                              mark_stack_limit, current_p, exit1);
+                              mark_stack_limit, current_p);
               }
             }
             descr <<= 1;
@@ -875,7 +875,7 @@ GC_INNER mse * GC_mark_from(mse *mark_stack_top, mse *mark_stack,
             }
 #         endif /* ENABLE_TRACE */
           PUSH_CONTENTS((ptr_t)current, mark_stack_top,
-                        mark_stack_limit, current_p, exit2);
+                        mark_stack_limit, current_p);
         }
         current_p += ALIGNMENT;
       }
@@ -892,7 +892,7 @@ GC_INNER mse * GC_mark_from(mse *mark_stack_top, mse *mark_stack,
             }
 #       endif /* ENABLE_TRACE */
         PUSH_CONTENTS((ptr_t)deferred, mark_stack_top,
-                      mark_stack_limit, current_p, exit4);
+                      mark_stack_limit, current_p);
         next_object:;
 #     endif
     }
@@ -1439,8 +1439,7 @@ GC_API struct GC_ms_entry * GC_CALL GC_mark_and_push(void *obj,
     }
 
     PUSH_CONTENTS_HDR(obj, mark_stack_ptr /* modified */, mark_stack_limit,
-                      (ptr_t)src, was_marked, hhdr, TRUE);
- was_marked:
+                      (ptr_t)src, hhdr, TRUE);
     return mark_stack_ptr;
 }
 
@@ -1484,8 +1483,7 @@ GC_API struct GC_ms_entry * GC_CALL GC_mark_and_push(void *obj,
       GC_dirty(p);      /* Implicitly affects entire object.            */
 #   endif
     PUSH_CONTENTS_HDR(r, GC_mark_stack_top, GC_mark_stack_limit,
-                      source, mark_and_push_exit, hhdr, FALSE);
-  mark_and_push_exit: ;
+                      source, hhdr, FALSE);
     /* We silently ignore pointers to near the end of a block,  */
     /* which is very mildly suboptimal.                         */
     /* FIXME: We should probably add a header word to address   */

diff --git a/typd_mlc.c b/typd_mlc.c
index 6b83cda9d91508ef4453e4fd2e827d05ebd53c4f..ecee04f9ad174f4027f0bb44c90b2e14c2353526 100644 (file)
--- a/typd_mlc.c
+++ b/typd_mlc.c
@@ -377,7 +377,7 @@ STATIC mse * GC_typed_mark_proc(word * addr, mse * mark_stack_ptr,
             FIXUP_POINTER(current);
             if (current >= (word)least_ha && current <= (word)greatest_ha) {
                 PUSH_CONTENTS((ptr_t)current, mark_stack_ptr,
-                              mark_stack_limit, (ptr_t)current_p, exit1);
+                              mark_stack_limit, (ptr_t)current_p);
             }
         }
     }