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

fix some segfaults while using riak_get() #44

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

folays
Copy link

@folays folays commented Feb 19, 2014

  • make the "not sync" operation of riak_get() really not async, thus not returing a NULL rop->response for an ERRIAK_OK status
  • fix some buffer overflows into functions called by riak_print_get_response()

root added 2 commits February 19, 2014 21:39
…ould return

 ERIAK_OK with a NULL rop->response, which would make riak_c_example segfault.
…o a buffer

- clean riak_object_print() and riak_links_print() to use riak_snprintf_cat()
- fix riak_print_get_response() buffer overflows and it's descendant
@bookshelfdave
Copy link
Contributor

thanks @folays, we'll try to get to this PR in the next couple of days.

@hazen
Copy link

hazen commented Feb 20, 2014

Good catches, @folays. Some of your changes might collide with #40, which is going to clean up the printing interface for all of the messages and structs. Some of the first ones which were written, like GET, are rather cumbersome as you already identified. Also we already have riak_print.c where most of the existing print routines live. We'd want to put new print routines in the same place. As @metadave mentioned, we'll look at it more carefully in the coming days.

@bookshelfdave
Copy link
Contributor

@folays - would you be wiling to submit a new PR to split out the bug fixes vs riak_snprintf_cat() changes? As @javajolt mentioned, we're working on some changes that will be part of #40, which will most likely collide with yours.

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

Successfully merging this pull request may close these issues.

3 participants