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

[From_CTB] ktable.get(wrong_length_string) gives core dump #174

Closed
RamRS opened this issue Oct 15, 2013 · 14 comments
Closed

[From_CTB] ktable.get(wrong_length_string) gives core dump #174

RamRS opened this issue Oct 15, 2013 · 14 comments
Labels
Milestone

Comments

@RamRS
Copy link
Contributor

RamRS commented Oct 15, 2013

Transferring #14 opened on 13-Sep-2012 (0 comments)

Example using latest khmer from github under 64bit Linux,

$ python
Python 2.7.3 (default, Sep 13 2012, 13:18:09) 
[GCC 4.4.6 20120305 (Red Hat 4.4.6-4)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import khmer
>>> ktable = khmer.new_ktable(6)
>>> ktable.consume("ATGAGAGACACAGGGAGAGACCCAATTAGAGAATTGGACC")
35
>>> ktable.get("ATGAGA")
1
>>> ktable.get("GATGAG")
0
>>> ktable.get("ATAGA")
python: ktable.cc:22: khmer::HashIntoType khmer::_hash(const char*, khmer::WordLength, khmer::HashIntoType&, khmer::HashIntoType&): Assertion `strlen(kmer) >= k' failed.
Aborted (core dumped)

I would expect the output to be zero, or perhaps a ValueError exception about the kmer length being wrong.

@mr-c
Copy link
Contributor

mr-c commented Oct 15, 2013

Huzzah: this test case now returns zero.

Unless @ctb or @peterjc have another test case then we can close this.

@RamRS
Copy link
Contributor Author

RamRS commented Mar 5, 2014

I might take a look into this next...

@luizirber
Copy link
Member

This looks related to #308

@RamRS
Copy link
Contributor Author

RamRS commented Mar 5, 2014

Yep. Maybe test once again after #308 is merged?

Ram

@luizirber
Copy link
Member

@mr-c required test cases for #308 before merging, it will happen sometime this week. I hope.

@RamRS
Copy link
Contributor Author

RamRS commented Mar 5, 2014

Cool, will watch out for it, then. Thank you, @luizirber !

@mr-c
Copy link
Contributor

mr-c commented Mar 19, 2014

The KTable class is unused by the current codebase. I purpose that it is removed.

@ctb what do you think?

@ctb
Copy link
Member

ctb commented Mar 20, 2014

On Wed, Mar 19, 2014 at 03:11:36PM -0700, Michael R. Crusoe wrote:

The KTable class is unused by the current codebase. I purpose that it is removed.

@ctb what do you think?

Fine by me.

@mr-c mr-c modified the milestones: 1.1+ Release, 1.0 release Apr 2, 2014
@mr-c
Copy link
Contributor

mr-c commented Aug 19, 2014

Note: table.get("wrong-sized-string") still blows up with table = khmer.new_hashbits(...), table = khmer.new_hashtable(...), and table = khmer.new_counting_hash(...)

In [3]: table = khmer.new_counting_hash(6,1e6)

In [4]: table.consume("ATGAGAGACACAGGGAGAGACCCAATTAGAGAATTGGACC")                                                                           
Out[4]: 35

In [5]: table.get("ATGAGA")                                                                                                                 
Out[5]: 1

In [6]: table.get("GATGAG")                                                                                                                
Out[6]: 0

In [7]: table.get("ATAGA")
terminate called after throwing an instance of 'khmer::khmer_exception'
  what():  Generic khmer exception
Aborted

@mr-c mr-c modified the milestones: 1.2 Release, 1.2+ Aug 19, 2014
@mr-c mr-c added the bug label Aug 19, 2014
@mr-c
Copy link
Contributor

mr-c commented Aug 20, 2014

When this is fixed the following should be done provide a better error message:

diff --git a/lib/kmer_hash.cc b/lib/kmer_hash.cc
index 403ef80..c0ed2c6 100644
--- a/lib/kmer_hash.cc
+++ b/lib/kmer_hash.cc
@@ -27,7 +27,7 @@ HashIntoType _hash(const char * kmer, const WordLength k,
 {
     // sizeof(HashIntoType) * 8 bits / 2 bits/base
     if (!(k <= sizeof(HashIntoType)*4) || !(strlen(kmer) >= k)) {
-        throw khmer_exception();
+        throw khmer_exception("Supplied kmer string doesn't match the underlying k-size.");
     }

     HashIntoType h = 0, r = 0;

@Echelon9
Copy link
Contributor

I've got a fix for this one. PR coming.

Echelon9 added a commit to Echelon9/khmer that referenced this issue Aug 28, 2014
Echelon9 added a commit to Echelon9/khmer that referenced this issue Aug 30, 2014
@Echelon9
Copy link
Contributor

Echelon9 commented Sep 1, 2014

This issue should now be resolved. Thanks @mr-c for merging the PR fix.

@mr-c mr-c closed this as completed Sep 1, 2014
@mr-c mr-c modified the milestones: 1.2 Release, 1.2+ Sep 1, 2014
@peterjc
Copy link

peterjc commented Dec 21, 2016

I originally filed this as #14 on ctb/khmer, but seems that got lost in the GitHub repository move as the new issue 14 is unrelated.

@ctb
Copy link
Member

ctb commented Dec 21, 2016

thx @peterjc - do we need to do anything here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants