Various collected fixes (updated versions of several existing PRs) #139
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The u_int32_t type comes from <sys/types.h>, which is often included as a byproduct of other headers on glibc platforms, but not on FreeBSD or Alpine Linux. Use <stdint.h>'s uint32_t instead, which is used elsewhere in bwa source code.
Fixes compilation on non-glibc platforms, e.g., FreeBSD and Alpine Linux.
Similarly to the realloc(pac,...) within add1(), only bother to call realloc() if appending the reverse complemented sequence requires more space than is currently in the pac/m_pac buffer. Avoids realloc(pac,0) (and a "Failed to allocate 0 bytes at bntseq.c" message from wrap_realloc()) in the corner case of an empty reference FASTA file. Fixes lh3#54.
[Based on PR lh3#37 with the additions below.] Don't free non-malloced items in BWTFree(). If BWTCreate() is ever called with a non-NULL decodeTable, BWTFree() will need to not free decodeTable -- see FIXME comment. Close packedFile in BWTIncConstructFromPacked() in the normal case. Ignore it in error cases, as they immediately call exit() anyway.
In the bwa.c and bwase.c calls, rlen is an int64_t returned from bns_get_seq() and is the number of reference bases covered by the alignment; l_query/len is an int and the query length of the alignment; and the result is an int given to an int parameter of ksw_global[2](). As even the result is int and as rlen is effectively bounded by the maximum length of a reference sequence, we maintain the status quo in this code and simply cast rlen to int to silence Clang's "use llabs()" (llabs() would not be a great answer given an int64_t anyway). The bwtsw2_pair.c call needs to remain fabs() so both divisions are done in floating point; cast to double to prevent Clang suggesting changing the call to integer abs().
As -o is still free in the mem command, use that standard option to specify an output file (and keep -f to parallel bwase/bwape commands). Document it in the usage and man page.
Correct header formatting, fix some typos in headers
This counter is only used in a log message. Fixes lh3#131.
FASTQ files containing NULs are invalid but should not cause bwa to crash, as it does if the quality line contains a NUL. Fixes lh3#122.
[Andreas Tille] Check exact number of arguments of bwtupdate Fix spelling [Fabian Klötzl] change printing as size_t is unsigned
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi Heng,
To facilitate merging I've collected and tidied up a number of pull requests into one larger PR. With one exception, these are just build and minor bug fixes and a README cleanup. For various of the proposed PRs, I've analysed the underlying problem and come up with what I believe is the best fix.
This PR supersedes the following:
FreeBSD / Alpine Linux build fixes; abs() warnings
Rather than adding more headers so
u_int32_tcan be used, just use the usualuint32_tinstead. Add explicit casts to theabs()/fabs()calls — see commit message for reasoning.Closes Minor changes to allow compilation under FreeBSD #33 (apart from the
CC=ccchange). Closes Fix warnings about types passed to abs() functions #68. Closes Missing includes for integral types #90. Closes incorrect use of abs intrinsic #130.Fix BWTIncFree() memory leak, and add to the previous PR so BWTFree() works properly. Closes Fixed BWTIncFree() memory leaks #37.
Change
bns_fasta2bntseqto only reallocpacif it needs to make it larger, in line with other bwa reallocing. Closes Fix for this error: "Failed to allocate 0 bytes at bntseq.c line 303" #54.Fix README.md headers. Closes Update README.md #70.
And adds some new minor fixes:
Fix log message
intoverflow. Fixes issue Log has MAX_INT issue for large input #131.Copy seq and qual even if they contain random data like NULs. Fixes issue Segmentation fault in bwamem.c #122.
Several trivial bug fixes from Debian.
This one is a small piece of new functionality:
bwa mem -o FILE/-f FILEoption to redirect output.-ois a widespread standard option for redirecting output, and luckily the-ooption letter is still available in bwa mem.-fas a synonym as it's familiar from other bwa subcommands. Closes Added -f option to write output to file in bwa-mem algorithm #41.Thanks for considering this.