Add extra warnings

The main things these picked up are fall-throughs on switch statements, all of which were deliberate. However they're now annotated as such so no longer complain. In samtools this did detect a genuine bug so it's worth adding (and annotating to silence FPs)
samtools · Aug 19, 2024 · bbbe4ee · bbbe4ee
1 parent 1654891
commit bbbe4ee
Show file tree

Hide file tree

Showing 5 changed files with 28 additions and 19 deletions.
diff --git a/.cirrus.yml b/.cirrus.yml
@@ -40,10 +40,10 @@ task:
   compile_script:
     - autoreconf -i
     - ./configure CC="clang" --disable-shared
-    - make -j4 CFLAGS="-g -O3 -Wall -Werror"
+    - make -j4 CFLAGS="-g -O3 -Wall -Werror -Wextra -Wno-sign-compare -Wno-unused-parameter"
 
   test_script:
-    - make check CFLAGS="-g -O3 -Wall -Werror"
+    - make check CFLAGS="-g -O3 -Wall -Werror -Wextra -Wno-sign-compare -Wno-unused-parameter"
     - make distcheck
 
 # Rocky Linux

diff --git a/htscodecs/pack.c b/htscodecs/pack.c
@@ -109,8 +109,8 @@ uint8_t *hts_pack(uint8_t *data, int64_t len,
         out[j] = 0;
         int s = len-i, x = 0;
         switch (s) {
-        case 3: out[j] |= p[data[i++]] << x; x+=2;
-        case 2: out[j] |= p[data[i++]] << x; x+=2;
+        case 3: out[j] |= p[data[i++]] << x; x+=2; // fall-through
+        case 2: out[j] |= p[data[i++]] << x; x+=2; // fall-through
         case 1: out[j] |= p[data[i++]] << x; x+=2;
             j++;
         }
@@ -125,12 +125,12 @@ uint8_t *hts_pack(uint8_t *data, int64_t len,
         out[j] = 0;
         int s = len-i, x = 0;
         switch (s) {
-        case 7: out[j] |= p[data[i++]] << x++;
-        case 6: out[j] |= p[data[i++]] << x++;
-        case 5: out[j] |= p[data[i++]] << x++;
-        case 4: out[j] |= p[data[i++]] << x++;
-        case 3: out[j] |= p[data[i++]] << x++;
-        case 2: out[j] |= p[data[i++]] << x++;
+        case 7: out[j] |= p[data[i++]] << x++; // fall-through
+        case 6: out[j] |= p[data[i++]] << x++; // fall-through
+        case 5: out[j] |= p[data[i++]] << x++; // fall-through
+        case 4: out[j] |= p[data[i++]] << x++; // fall-through
+        case 3: out[j] |= p[data[i++]] << x++; // fall-through
+        case 2: out[j] |= p[data[i++]] << x++; // fall-through
         case 1: out[j] |= p[data[i++]] << x++;
             j++;
         }

diff --git a/htscodecs/rANS_static.c b/htscodecs/rANS_static.c
@@ -167,8 +167,11 @@ unsigned char *rans_compress_O0(unsigned char *in, unsigned int in_size,
 
     switch (i=(in_size&3)) {
     case 3: RansEncPutSymbol(&rans2, &ptr, &syms[in[in_size-(i-2)]]);
+        // fall-through
     case 2: RansEncPutSymbol(&rans1, &ptr, &syms[in[in_size-(i-1)]]);
+        // fall-through
     case 1: RansEncPutSymbol(&rans0, &ptr, &syms[in[in_size-(i-0)]]);
+        // fall-through
     case 0:
         break;
     }
@@ -361,10 +364,13 @@ unsigned char *rans_uncompress_O0(unsigned char *in, unsigned int in_size,
     switch(out_sz&3) {
     case 3:
         out_buf[out_end + 2] = ssym[R[2] & mask];
+        // fall-through
     case 2:
         out_buf[out_end + 1] = ssym[R[1] & mask];
+        // fall-through
     case 1:
         out_buf[out_end] = ssym[R[0] & mask];
+        // fall-through
     default:
         break;
     }

diff --git a/htscodecs/rANS_static4x16pr.c b/htscodecs/rANS_static4x16pr.c
@@ -176,8 +176,11 @@ unsigned char *rans_compress_O0_4x16(unsigned char *in, unsigned int in_size,
 
     switch (i=(in_size&3)) {
     case 3: RansEncPutSymbol(&rans2, &ptr, &syms[in[in_size-(i-2)]]);
+        // fall-through
     case 2: RansEncPutSymbol(&rans1, &ptr, &syms[in[in_size-(i-1)]]);
+        // fall-through
     case 1: RansEncPutSymbol(&rans0, &ptr, &syms[in[in_size-(i-0)]]);
+        // fall-through
     case 0:
         break;
     }

diff --git a/htscodecs/tokenise_name3.c b/htscodecs/tokenise_name3.c
@@ -232,15 +232,15 @@ static void free_context(name_context *ctx) {
 // Returns number of bytes written.
 static int append_uint32_fixed(char *cp, uint32_t i, uint8_t l) {
     switch (l) {
-    case 9:*cp++ = i / 100000000 + '0', i %= 100000000;
-    case 8:*cp++ = i / 10000000  + '0', i %= 10000000;
-    case 7:*cp++ = i / 1000000   + '0', i %= 1000000;
-    case 6:*cp++ = i / 100000    + '0', i %= 100000;
-    case 5:*cp++ = i / 10000     + '0', i %= 10000;
-    case 4:*cp++ = i / 1000      + '0', i %= 1000;
-    case 3:*cp++ = i / 100       + '0', i %= 100;
-    case 2:*cp++ = i / 10        + '0', i %= 10;
-    case 1:*cp++ = i             + '0';
+    case 9:*cp++ = i / 100000000 + '0', i %= 100000000; // fall-through
+    case 8:*cp++ = i / 10000000  + '0', i %= 10000000;  // fall-through
+    case 7:*cp++ = i / 1000000   + '0', i %= 1000000;   // fall-through
+    case 6:*cp++ = i / 100000    + '0', i %= 100000;    // fall-through
+    case 5:*cp++ = i / 10000     + '0', i %= 10000;     // fall-through
+    case 4:*cp++ = i / 1000      + '0', i %= 1000;      // fall-through
+    case 3:*cp++ = i / 100       + '0', i %= 100;       // fall-through
+    case 2:*cp++ = i / 10        + '0', i %= 10;        // fall-through
+    case 1:*cp++ = i             + '0';                 // fall-throuhg
     case 0:break;
     }
     return l;