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

Fix hfile_libcurl small seek bug #1676

Merged
merged 1 commit into from
Sep 26, 2023
Merged

Conversation

daviesrob
Copy link
Member

Ensure that to_skip is updated correctly when handling a small seek by skipping data in libcurl_read(). In particular, if the number of bytes returned by wait_perform() exactly matches the number left to skip, to_skip needs to be set to zero, and wait_perform() needs to be called again to get some data for the function to return.

Fixes samtools/samtools#1918; possibly also #1037, #1625 and samtools/samtools#1622

@jmarshall I've made you author of ths commit, as it was your patch. Let me know if you'd like to change anything before it gets merged.

@jmarshall
Copy link
Member

The commit and the explanation in the commit message look good.

IMHO you should add a Co-Authored-By: RMD — I just tried a <= tweak and verified that it fixed Alex's problem, but didn't look into how got and to_skip work. You've added the understanding and explanation that this is indeed the right fix in general.

Ensure that `to_skip` is updated correctly when handling a
small seek by skipping data in libcurl_read().  In particular,
if the number of bytes returned by wait_perform() exactly
matches the number left to skip, `to_skip` needs to be set to zero,
and wait_perform() needs to be called again to get some data for
the function to return.

Co-Authored-By: Rob Davies <rmd+git@sanger.ac.uk>
@daviesrob
Copy link
Member Author

Cheers. I've updated the commit message as suggested.

@daviesrob daviesrob merged commit f0426df into samtools:develop Sep 26, 2023
8 checks passed
@daviesrob daviesrob deleted the small_seek branch September 26, 2023 14:14
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.

Querying of HTTPS data via samtools v1.18/htslib v1.18 hangs
2 participants