sickle icon indicating copy to clipboard operation
sickle copied to clipboard

segfault on empty reads

Open roryk opened this issue 11 years ago • 10 comments

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?

roryk avatar Aug 13 '14 19:08 roryk

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

wookietreiber avatar Aug 13 '14 22:08 wookietreiber

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.

roryk avatar Aug 15 '14 03:08 roryk

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

wookietreiber avatar Aug 15 '14 05:08 wookietreiber

@najoshi any chance of getting this merged in?

roryk avatar Feb 24 '15 19:02 roryk

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 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)

najoshi avatar Feb 25 '15 00:02 najoshi

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

tsibley avatar Feb 25 '15 00:02 tsibley

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.

ucdavis-bioinformatics avatar Feb 25 '15 03:02 ucdavis-bioinformatics

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 avatar Feb 25 '15 09:02 wookietreiber

Fix now in #33

wookietreiber avatar Feb 25 '15 11:02 wookietreiber

Hi @najoshi and @wookietreiber,

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

roryk avatar Feb 25 '15 13:02 roryk