Skip to content

should bam::Record::push_aux be infallible? #147

@pmarks

Description

@pmarks

The only way that push_aux can fail is on OOM, or if an invalid type code is passed. The Aux enum should prevent an invalid type code from being passed. There are a bunch of other functions (eg set_qname and related) that can fail due to OOM, but don't propagate an error. I find myself adding a lot of error handling to non-IO-containing code, just to propagate errors from push_aux. I think we should remove the Result<> return, even though it would be a breaking change.

push_aux impl:
https://github.com/samtools/htslib/blob/673813297cee2c3235a41f9406ad2aa9494de954/sam.c#L2246

Note: in looking at the set_qname code path, I discovered that there's a memory safety bug here:
https://docs.rs/rust-htslib/0.24.0/src/rust_htslib/bam/record.rs.html#395
If realloc fails it will return NULL. We should check that condition and panic, rather than proceeding. I'll make a PR for that.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions