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

segfault on empty reads #32

Open
roryk opened this issue Aug 13, 2014 · 10 comments
Open

segfault on empty reads #32

roryk opened this issue Aug 13, 2014 · 10 comments

Comments

@roryk
Copy link

roryk commented Aug 13, 2014

Hi @najoshi,

I'm using sickle to clean up some trimming with cutadapt and some of the reads coming from cutadapt can have length zero, which causes sickle to segfault. I put up a file with two reads in it, one of which is empty, as an example here:

https://dl.dropboxusercontent.com/u/2822886/one_empty_read.tar

If you drop the empty read it works fine so I think it is due to the read being empty.

Any shot at a fix?

wookietreiber added a commit to idiv-biodiversity/sickle that referenced this issue Aug 13, 2014
@wookietreiber
Copy link

Failing line is this one:

seq->seq.s[seq->seq.l] = 0; /* null terminated string */ \

@roryk
Copy link
Author

roryk commented Aug 15, 2014

Thanks Christian,

Do you know what the status of your pull request is? This has been open for a couple of weeks with no action.

@wookietreiber
Copy link

@najoshi has not been active since Jul 26, so I don't know, maybe on holiday.

@roryk
Copy link
Author

roryk commented Feb 24, 2015

@najoshi any chance of getting this merged in?

@najoshi
Copy link
Owner

najoshi commented Feb 25, 2015

Hi Rory,

As far as I can tell, all this merge would do is add gnu autotools build to
the repo, and it doesn't seem like it actually fixes that problem, just
tests for it. Also, kseq.h is written by Heng Li, not me, so I'm not sure
of the best way to fix it.

  • Nik.

On Tue, Feb 24, 2015 at 11:58 AM, Rory Kirchner [email protected]
wrote:

@najoshi https://github.com/najoshi any chance of getting this merged
in?


Reply to this email directly or view it on GitHub
#32 (comment).

Nikhil Joshi
Bioinformatics Analyst/Programmer
UC Davis Bioinformatics Core
http://bioinformatics.ucdavis.edu/
najoshi -at- ucdavis -dot- edu
530.752.2698 (w)

@tsibley
Copy link

tsibley commented Feb 25, 2015

Perhaps an updated copy of kseq.h would do the trick, though this is just a shot in the dark. I haven't looked at the code.

Latest version from htslib: https://github.com/samtools/htslib/blob/develop/htslib/kseq.h

@ucdavis-bioinformatics
Copy link
Contributor

So I think I've fixed it. I took some code from the new kseq.h and put it into this one to deal with the empty read problem. Test it out and let me know if you find more bugs.

@wookietreiber
Copy link

Hi @najoshi, good to finally hear from you.

The reason I added a GNU autotools build is that it makes it much easier to add some tests and automatically execute them (as well as build and install the software in a portable and standard way).

Yes, I did not have had a fix yet, just implemented the test to check the issue.

@ucdavis-bioinformatics Maybe you can base your fix upon my #33 PR?

@wookietreiber
Copy link

Fix now in #33

@roryk
Copy link
Author

roryk commented Feb 25, 2015

Hi @najoshi and @wookietreiber,

Thanks for the responses-- the fix in master works on the test file I posted. #33 does as well.

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

No branches or pull requests

4 participants