From f245fc2b6aaca534c77156ccaa5fa33d026cd1f9 Mon Sep 17 00:00:00 2001
From: James Bonfield <jkb@sanger.ac.uk>
Date: Mon, 1 Jul 2024 14:46:25 +0100
Subject: [PATCH] Fix a missing bgzf->uncompressed_address incr in
 bgzf_read_small

This bug crept in with #1772 which was added since last release, so
there is no regression.

Fixes #1798 with thanks to John Marshall
---
 htslib/bgzf.h    |  4 ++-
 test/test_bgzf.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 82 insertions(+), 1 deletion(-)

diff --git a/htslib/bgzf.h b/htslib/bgzf.h
index 886c60767b..c6ce7c1725 100644
--- a/htslib/bgzf.h
+++ b/htslib/bgzf.h
@@ -157,6 +157,7 @@ static inline ssize_t bgzf_read_small(BGZF *fp, void *data, size_t length) {
                (uint8_t *)fp->uncompressed_block + fp->block_offset,
                length);
         fp->block_offset += length;
+        fp->uncompressed_address += length;
         return length;
     } else {
         return bgzf_read(fp, data, length);
@@ -179,7 +180,8 @@ static inline ssize_t bgzf_read_small(BGZF *fp, void *data, size_t length) {
  * bgzf_write optimised for small quantities, as a static inline
  * See bgzf_write() normal function for return values.
  */
-static inline ssize_t bgzf_write_small(BGZF *fp, void *data, size_t length) {
+static inline
+ssize_t bgzf_write_small(BGZF *fp, const void *data, size_t length) {
     if (fp->is_compressed && BGZF_BLOCK_SIZE - fp->block_offset > length) {
         // Short cut the common and easy mode
         memcpy((uint8_t *)fp->uncompressed_block + fp->block_offset,
diff --git a/test/test_bgzf.c b/test/test_bgzf.c
index 6cb6db902f..bf425520b3 100644
--- a/test/test_bgzf.c
+++ b/test/test_bgzf.c
@@ -40,6 +40,15 @@ DEALINGS IN THE SOFTWARE.
 #include "../htslib/hts_log.h"
 #include "../hfile_internal.h"
 
+// Trial also testing everything using bgzf_read_small and bgzf_write_small,
+// These degrade back to bgzf_read and bgzf_write for larger blocks or those
+// that span blocks anyway, so tests both APIs.
+//
+// Alternatively we could craft extra chekcs or parameterise existing ones.
+// For now though, this is a simpler check.
+#define bgzf_read bgzf_read_small
+#define bgzf_write bgzf_write_small
+
 const char *bgzf_suffix = ".gz";
 const char *idx_suffix  = ".gzi";
 const char *tmp_suffix  = ".tmp";
@@ -187,6 +196,16 @@ static ssize_t try_bgzf_read(BGZF *fp, void *data, size_t length,
     return got;
 }
 
+static ssize_t try_bgzf_read_small(BGZF *fp, void *data, size_t length,
+                                   const char *name, const char *func) {
+    ssize_t got = bgzf_read_small(fp, data, length);
+    if (got < 0) {
+        fprintf(stderr, "%s : Error from bgzf_read %s : %s\n",
+                func, name, strerror(errno));
+    }
+    return got;
+}
+
 static ssize_t try_bgzf_write(BGZF *fp, const void *data, size_t length,
                               const char *name, const char *func) {
     ssize_t put = bgzf_write(fp, data, length);
@@ -200,6 +219,19 @@ static ssize_t try_bgzf_write(BGZF *fp, const void *data, size_t length,
     return put;
 }
 
+static ssize_t try_bgzf_write_small(BGZF *fp, const void *data, size_t length,
+                                    const char *name, const char *func) {
+    ssize_t put = bgzf_write_small(fp, data, length);
+    if (put < (ssize_t) length) {
+        fprintf(stderr, "%s : %s %s : %s\n",
+                func, put < 0 ? "Error writing to" : "Short write on",
+                name, strerror(errno));
+        return -1;
+    }
+
+    return put;
+}
+
 static int try_bgzf_compression(BGZF *fp, int expect,
                                 const char *name, const char *func) {
     int res = bgzf_compression(fp);
@@ -878,6 +910,49 @@ static int test_tell_read(Files *f, const char *mode) {
     return -1;
 }
 
+static int test_useek_read_small(Files *f, const char *mode) {
+
+    BGZF* bgz = NULL;
+    char bg_buf[99];
+
+    bgz = try_bgzf_open(f->tmp_bgzf, mode, __func__);
+    if (!bgz) goto fail;
+
+
+    if (try_bgzf_write_small(bgz, "#>Hello, World!\n", 16,
+                             f->tmp_bgzf, __func__) != 16)
+        goto fail;
+    if (try_bgzf_close(&bgz, f->tmp_bgzf, __func__, 0) != 0) goto fail;
+
+    bgz = try_bgzf_open(f->tmp_bgzf, "r", __func__);
+    if (!bgz) goto fail;
+
+    if (try_bgzf_getc(bgz, 0, '#', f->tmp_bgzf, __func__) < 0 ||
+        try_bgzf_getc(bgz, 1, '>', f->tmp_bgzf, __func__) < 0)
+        goto fail;
+
+    if (try_bgzf_read_small(bgz, bg_buf, 5, f->tmp_bgzf, __func__) != 5)
+        goto fail;
+    if (memcmp(bg_buf, "Hello", 5) != 0)
+        goto fail;
+
+    if (try_bgzf_useek(bgz, 9, SEEK_SET, f->tmp_bgzf, __func__) < 0)
+        goto fail;
+
+    if (try_bgzf_read_small(bgz, bg_buf, 5, f->tmp_bgzf, __func__) != 5)
+        goto fail;
+    if (memcmp(bg_buf, "World", 5) != 0)
+        goto fail;
+
+    if (try_bgzf_close(&bgz, f->tmp_bgzf, __func__, 0) != 0) goto fail;
+    return 0;
+
+ fail:
+    fprintf(stderr, "%s: failed\n", __func__);
+    if (bgz) bgzf_close(bgz);
+    return -1;
+}
+
 static int test_bgzf_getline(Files *f, const char *mode, int nthreads) {
     BGZF* bgz = NULL;
     ssize_t bg_put;
@@ -1098,6 +1173,10 @@ int main(int argc, char **argv) {
     if (test_tell_read(&f, "w") != 0) goto out;
     if (test_tell_read(&f, "wu") != 0) goto out;
 
+    // bgzf_useek and bgzf_read_small
+    if (test_useek_read_small(&f, "w") != 0) goto out;
+    if (test_useek_read_small(&f, "wu") != 0) goto out;
+
     // getline
     if (test_bgzf_getline(&f, "w", 0) != 0) goto out;
     if (test_bgzf_getline(&f, "w", 1) != 0) goto out;