najoshi / sickle

Windowed Adaptive Trimming for fastq files using quality
MIT License
219 stars 95 forks source link

segfault on empty reads #32

Open roryk opened 10 years ago

roryk commented 10 years ago

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 commented 10 years ago

Failing line is this one: https://github.com/najoshi/sickle/blob/7667f147e6e00e86109e69b973059dea40c6f23e/src/kseq.h#L194

roryk commented 10 years ago

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 commented 10 years ago

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

roryk commented 9 years ago

@najoshi any chance of getting this merged in?

najoshi commented 9 years ago

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.

On Tue, Feb 24, 2015 at 11:58 AM, Rory Kirchner notifications@github.com wrote:

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

— Reply to this email directly or view it on GitHub https://github.com/najoshi/sickle/issues/32#issuecomment-75833196.

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

tsibley commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

Fix now in #33

roryk commented 9 years ago

Hi @najoshi and @wookietreiber,

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