Modifications following review

Author: jkbonfield

htslib/vcf.h

@@ -1532,7 +1532,7 @@ static inline int bcf_enc_size(kstring_t *s, int size, int type)
         return 0;
     }
 
-    if (ks_resize(s, s->l + 5) < 0)
+    if (ks_resize(s, s->l + 6) < 0)
         return -1;
     uint8_t *p = (uint8_t *)s->s + s->l;
     *p++ = 15<<4|type;
@@ -1555,6 +1555,13 @@ static inline int bcf_enc_size(kstring_t *s, int size, int type)
     return 0;
 }
 
+static inline int bcf_enc_inttype(long x)
+{
+    if (x <= BCF_MAX_BT_INT8 && x >= BCF_MIN_BT_INT8) return BCF_BT_INT8;
+    if (x <= BCF_MAX_BT_INT16 && x >= BCF_MIN_BT_INT16) return BCF_BT_INT16;
+    return BCF_BT_INT32;
+}
+
 // INTERNAL - not to be assumed to be a part of public API
 // As per bcf_enc_size but kstring is already guaranteed to be big enough
 // and with size == 1.
@@ -1565,35 +1572,36 @@ static inline void bcf_enc_size1_(kstring_t *s, int type)
         s->l++;
 }
 
-static inline int bcf_enc_inttype(long x)
-{
-    if (x <= BCF_MAX_BT_INT8 && x >= BCF_MIN_BT_INT8) return BCF_BT_INT8;
-    if (x <= BCF_MAX_BT_INT16 && x >= BCF_MIN_BT_INT16) return BCF_BT_INT16;
-    return BCF_BT_INT32;
-}
-
 static inline int bcf_enc_int1(kstring_t *s, int32_t x)
 {
     if (ks_resize(s, s->l + 5) < 0)
         return -1;
-    uint8_t *p = (uint8_t *)s->s + s->l + 1/*1 for size*/;
+    uint8_t *p = (uint8_t *)s->s + s->l;
 
     if (x == bcf_int32_vector_end) {
-        bcf_enc_size1_(s, BCF_BT_INT8);
-        *p = bcf_int8_vector_end; s->l++;
+        // An inline implementation of bcf_enc_size with size==1 and
+        // memory allocation already accounted for.
+        *p = (1<<4) | BCF_BT_INT8;
+        p[1] = bcf_int8_vector_end;
+        s->l+=2;
     } else if (x == bcf_int32_missing) {
-        bcf_enc_size1_(s, BCF_BT_INT8);
-        *p = bcf_int8_missing; s->l++;
+        *p = (1<<4) | BCF_BT_INT8;
+        p[1] = bcf_int8_missing;
+        s->l+=2;
     } else if (x <= BCF_MAX_BT_INT8 && x >= BCF_MIN_BT_INT8) {
-        bcf_enc_size1_(s, BCF_BT_INT8);
-        *p = x; s->l++;
+        *p = (1<<4) | BCF_BT_INT8;
+        p[1] = x;
+        s->l+=2;
     } else if (x <= BCF_MAX_BT_INT16 && x >= BCF_MIN_BT_INT16) {
-        bcf_enc_size1_(s, BCF_BT_INT16);
-        i16_to_le(x, p); s->l+=2;
+        *p = (1<<4) | BCF_BT_INT16;
+        i16_to_le(x, p+1);
+        s->l+=3;
     } else {
-        bcf_enc_size1_(s, BCF_BT_INT32);
-        i32_to_le(x, p); s->l+=4;
+        *p = (1<<4) | BCF_BT_INT32;
+        i32_to_le(x, p+1);
+        s->l+=5;
     }
+
     return 0;
 }
 

vcf.c

@@ -3512,6 +3512,16 @@ int vcf_parse(kstring_t *s, const bcf_hdr_t *h, bcf1_t *v)
     if (ks_resize(s, s->l+4) < 0)
         return -1;
 
+    // Force our memory to be initialised so we avoid the technicality of
+    // undefined behaviour in using a 4-byte memcmp.  (The reality is this
+    // almost certainly is never detected by the compiler so has no impact,
+    // but equally so this code has minimal (often beneficial) impact on
+    // performance too.)
+    s->s[s->l+0] = 0;
+    s->s[s->l+1] = 0;
+    s->s[s->l+2] = 0;
+    s->s[s->l+3] = 0;
+
     bcf_clear1(v);
     str = &v->shared;
     memset(&aux, 0, sizeof(ks_tokaux_t));