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

Various minor fixes #238

Closed
wants to merge 17 commits into from
Closed

Various minor fixes #238

wants to merge 17 commits into from

Conversation

gtamazian
Copy link
Contributor

The fixes were motivated by the clang C++ compiler warnings.

@@ -148,7 +148,7 @@ std::FILE* BedSplit::saveFileChunk(std::string& filename,size_t file_index)

char tmp[10];
filename.assign(this->outfileprefix);
sprintf(tmp,"%05d",(file_index+1));
sprintf(tmp,"%05lu",(file_index+1));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

%05zu; but probably it would be best to just avoid the issue by building the filename via an ostringstream.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I fixed it in f9001c5.

@jmarshall
Copy link
Contributor

PR #233 has a better fix for the overflow warning that makes int2str() work correctly for large numbers.

@arq5x
Copy link
Owner

arq5x commented May 15, 2015

Thanks all. We will try to merge in all of these patches soon.

@gtamazian
Copy link
Contributor Author

Fixed some tests; see also PR #239 for fixes of bedtools coverage and bedtools sample tests. Also possible overflows were fixed for bedtools slop in 5e017b6.

If modification time of a FASTA index file is less than modification
time of its FASTA file, then the warning message is shown; resolves
#228.
@gtamazian gtamazian changed the title Minor fixes inspired by compiler warnings Various minor fixes May 21, 2015
To make the test more robust, the original FASTA file is replaced with
another one with the same sequences but different headers. I used the
example by @dshivak.
@gtamazian
Copy link
Contributor Author

Changes from this pull request were split to several branches for more convenient review; please check PR #249, #250 and #251.

@gtamazian gtamazian closed this Jun 2, 2015
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