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

Add some docs + unit tests #115

Merged
merged 1 commit into from
Oct 31, 2016
Merged

Add some docs + unit tests #115

merged 1 commit into from
Oct 31, 2016

Conversation

ret2libc
Copy link
Contributor

No description provided.

@radare
Copy link
Collaborator

radare commented Oct 26, 2016

looks red

@ret2libc
Copy link
Contributor Author

As I told you yesterday, with the last commits you just hide the bug of key_len. This new tests show it again (in particular at line https://github.com/radare/sdb/pull/115/files#diff-8ed9a6a44ab7365326573ab0e2597e65R118 )

@radare
Copy link
Collaborator

radare commented Oct 26, 2016

dont push a breaking test without the fix

On 26 Oct 2016, at 10:28, Riccardo Schirone notifications@github.com wrote:

As I told you yesterday, with the last commits you just hide the bug of key_len. This new tests show it again (in particular at line https://github.com/radare/sdb/pull/115/files#diff-8ed9a6a44ab7365326573ab0e2597e65R118 https://github.com/radare/sdb/pull/115/files#diff-8ed9a6a44ab7365326573ab0e2597e65R118 )


You are receiving this because you commented.
Reply to this email directly, view it on GitHub #115 (comment), or mute the thread https://github.com/notifications/unsubscribe-auth/AA3-ltJygBkjxPQI_XPbauONlA1Zi9Foks5q3w8RgaJpZM4Kgfut.

@ret2libc
Copy link
Contributor Author

let's bury our heads in the sand then :P i will write more tests... then we can see how to fix things

@radare
Copy link
Collaborator

radare commented Oct 27, 2016

http://i1.kym-cdn.com/entries/icons/original/000/018/012/Screen_Shot_2015-05-12_at_3.31.31_PM.png http://i1.kym-cdn.com/entries/icons/original/000/018/012/Screen_Shot_2015-05-12_at_3.31.31_PM.png

On 26 Oct 2016, at 23:19, Riccardo Schirone notifications@github.com wrote:

let's bury our heads in the sand then :P i will write more tests... then we can see how to fix things


You are receiving this because you commented.
Reply to this email directly, view it on GitHub #115 (comment), or mute the thread https://github.com/notifications/unsubscribe-auth/AA3-lt1MQHO0TSfBmdRE3EdvutWGFt2_ks5q38PUgaJpZM4Kgfut.

@radare
Copy link
Collaborator

radare commented Oct 31, 2016

can you fix that issue in the same pr?

@radare
Copy link
Collaborator

radare commented Oct 31, 2016

i know this change implies asuming that length doesnt contains the tailing \0

@radare radare merged commit 78f5ee4 into radareorg:master Oct 31, 2016
@radare
Copy link
Collaborator

radare commented Oct 31, 2016

tired of waiting. will fix it myself

@radare
Copy link
Collaborator

radare commented Oct 31, 2016

see 2d45c1c it was just 1 loc

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.

2 participants