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

Proposal to add hts_log callback function and hts_log_set_logger() #1414

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

jmarshall
Copy link
Member

A somewhat tidied-up version of @lczech's concrete proposal in #1397 for overriding the default printing of hts_log messages to stderr. Draft, as the new function (and associated typedef) would require documenting in hts_log.h.

I don't know whether the hts_log_* functions are called from threads that HTSlib has started. If so, this callback function would be called from those threads — which might be “interesting”. This PR doesn't consider what to do in this case.

[DRAFT] Needs documentation in htslib/hts_log.h.

Co-Authored-By: John Marshall <[email protected]>
@daviesrob
Copy link
Member

I think this looks OK in principle, although we should do something about the (not unlikely) possibility of the hts_logger() callback being called from a thread. I think at the least, this should be mentioned in the callback documentation. It would also be a good idea to add a mutex, to be held both when setting the callback function and when calling it. That would both eliminate the small risk of data races, and allow us to guarantee that the callback will not be called simultaneously from different threads.

@jmarshall
Copy link
Member Author

@lczech or the maintainers should feel free to do so, and may find this PR (or just the commit) a useful starting point.

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.

3 participants