Skip to content

Commit

Permalink
Fix a missing bgzf->uncompressed_address incr in bgzf_read_small
Browse files Browse the repository at this point in the history
This bug crept in with #1772 which was added since last release, so
there is no regression.

Fixes #1798 with thanks to John Marshall
  • Loading branch information
jkbonfield committed Jul 1, 2024
1 parent 61b922b commit f245fc2
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 1 deletion.
4 changes: 3 additions & 1 deletion htslib/bgzf.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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,
Expand Down
79 changes: 79 additions & 0 deletions test/test_bgzf.c
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit f245fc2

Please sign in to comment.