samtools / htslib

C library for high-throughput sequencing data formats
Other
783 stars 447 forks source link

Use only _regions_add() in synced_bcf_reader.c when adding the list of contig names #1781

Closed jmarshall closed 1 month ago

jmarshall commented 1 month ago

Quoting from samtools/bcftools#2179:

As the in the ##contig=<ID=…> lines unambiguously denotes a contig name (and there are no region strings anywhere near ID in the grammar, such as it is), passing this to _regions_init_string() as is is clearly a bcftools (or htslib) bug. Whatever the code is trying to do, it needs to do it by calling a dumber function that accepts only a contig name […]

The dumber function already exists and is already used by this code for all but the first contig header line. The code used _regions_init_string() rather than _regions_add() only when needed the first time to allocate a new bcf_sr_regions_t structure; instead extract basic initialisation into a new bcf_sr_regions_alloc() function and use _regions_add() exclusively.

As a bonus, the new function actually checks that the memory allocation succeeded. Use the new function throughout.

Fixes samtools/bcftools#2179.

jkbonfield commented 1 month ago

Thanks John