Skip to content

Improve synced reader error checking#2024

Open
daviesrob wants to merge 3 commits into
samtools:developfrom
daviesrob:synced-reader-updates
Open

Improve synced reader error checking#2024
daviesrob wants to merge 3 commits into
samtools:developfrom
daviesrob:synced-reader-updates

Conversation

@daviesrob
Copy link
Copy Markdown
Member

Mainly adds tests to ensure that memory allocations worked, along with places where hts_getline() failure may have been missed before. Ensures that errors propagate up to callers, and that bcf_srs_t::errnum is set consistently.

The bcf_sr_error enumerated type used for bcf_srs_t::errnum gets two new entries bcf_sr_seek_error and bcf_sr_regions_error for cases not really covered by the existing values. As the old open_failed entry unfortunately had the value zero, which meant it looked like "no error" in the recommended way of checking bcf_srs_t::errnum, it has been deprecated and replaced by bcf_sr_open_failed. A bcf_sr_ok entry is also added with value zero to make the "no errors" case explicit.

Where possible, structures are left in a consistent state when recovering from an allocation failure. For example, fields recording how much memory was allocated are only updated after realloc() succeeds, so it does not appear that more space is available than is really the case.

Fixes a couple of places where the synced reader would call exit(1) rather than return an error code to the caller. This does mean callers may have to become better at checking return values.

The synced reader documentation also gets some updates to make it more compatible with doxygen, and to update descriptions of the values returned by some functions.

daviesrob added 3 commits May 28, 2026 16:57
As enum	bcf_sr_error didn't explicitly set an values, the
first entry (open_failed) got the value zero.  As a check
against zero is suggested in example code (and also used
frequently in bcftools) to test for errors, it was not
possible to distinguish open_failed from no error.  Fix
by deprecating open_failed and replacing it by a new
non-zero value `bcf_sr_open_failed` to cover the case.

A new entry bcf_sr_ok is added to make the OK case explicit.

New entries bcf_sr_seek_error, bcf_sr_regions_error, and
bcf_sr_samples_error are added for use by later updates.

Signed-off-by: Rob Davies <rmd+git@sanger.ac.uk>
Mainly adds tests to ensure that memory allocations worked,
along with places where hts_getline() failure may have been
missed before.  Ensures that errors propagate up to callers,
and that bcf_srs_t::errnum is set consistently.

Where possible, structures are left in a consistent state
when recovering from an allocation failure.  For example,
fields recording how much memory was allocated are only
updated after realloc() succeeds, so it does not appear that
more space is available than is really the case.

Fixes a couple of places where the synced reader would call
exit(1) rather than return an error code to the caller.  This
does mean callers may have to become better at checking return
values.

Signed-off-by: Rob Davies <rmd+git@sanger.ac.uk>
Ensure return values are documented.

Convert markup to better match doxygen format.

Signed-off-by: Rob Davies <rmd+git@sanger.ac.uk>
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.

1 participant