Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
May 9, 2024

Conversation

jmarshall
Copy link
Member

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.

Don't use _regions_init_string(), which misinterprets contig names
containing colons as region specification strings. The code used
_regions_init_string() rather than _regions_add() only when needed
to allocate a new bcf_sr_regions_t structure; instead extract basic
initialisation into a new bcf_sr_regions_alloc() function, which as
a bonus checks the memory allocation. Use the new function throughout.

Fixes samtools/bcftools#2179.
@jkbonfield jkbonfield merged commit 9ad8270 into samtools:develop May 9, 2024
9 checks passed
@jkbonfield
Copy link
Contributor

Thanks John

@jmarshall jmarshall deleted the regions_add branch May 10, 2024 04:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Colons in VCF #CHROM columns are not parsed and throw error
2 participants