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

Protocol buffers conversion, packaging, GLib error handling #4

Open
wants to merge 22 commits into
base: master
Choose a base branch
from

Conversation

dhull
Copy link

@dhull dhull commented Dec 15, 2011

I have adopted riak-c-driver for deployment in a C application that writes JSON data to a Riak database, and these commits cover the changes that I made for that work.

I converted the record get/put code to use protocol buffers. I wanted to use binary keys in my application, so I added functions that take a length parameter because my keys might contain NUL bytes.

I packaged the code with the GNU autotools, and added a pkgconfig file. I added support for building RPMs (We're using CentOS).

I changed the code to use GLib's GError for error reporting. I updated the code toto use the GLib memory allocation functions and got rid of all fixed-length buffers.

I rewrote the riak_exec_op code to use fewer system calls.

David Hull added 22 commits December 13, 2011 10:20
Remove connstruct argument from riak_init.
Add riak_putb_json which takes binary bucket and key.
Add riak_getb_raw and riak_get_raw, which fetches a value from Riak.
Fix fixed buffer size in writefunc.
* Don't need to check for memory allocation failure with glib malloc
  functions.

* Don't need to worry about overflowing fixed-length buffers with
  g_strdup_printf.
Remove the "_raw" from riak_getb_raw and riak_get_raw be consistent
with riak_putb and riak_put.
@fenek
Copy link
Owner

fenek commented Jan 30, 2012

Hello,

Sorry for a very late response, I'm simply overwhelmed by university and professional work. ;) I'm looking through your changes now and I have one question: you've decided to use GError to error reporting mainly because of integration with a package for CentOS or something else was a main reason?
Besides, I really appreciate creating autoconf and automake scripts, I never really had an occasion to learn how to write them and this is of a great help!

Back to studying your changes... ;)

@dhull
Copy link
Author

dhull commented Jan 31, 2012

On Jan 30, 2012, at 4:34 AM, Piotr Nosek wrote:

Sorry for a very late response, I'm simply overwhelmed by university and professional work. ;)

No problem.

I'm looking through your changes now and I have one question: you've decided to use GError to error reporting mainly because of integration with a package for CentOS or something else was a main reason?

I originally pulled in GLib to fix some problems where the code wasn't checking the return value of malloc or was sprintf'ing into a fixed-length buffer. Since I had already created the dependency on GLib, I thought it would be worth making the error reporting between the various functions more consistent.

When I was doing this work I was working on a project for which we were going to use Riak. The part written in C was going to be doing inserts to Riak using a binary key. (The actual key was a URL but the database was going to have many keys so we took the SHA1 of the key to reduce the in-memory size of the database.) That's why I added the interfaces for non-null-terminated keys. Unfortunately the project was canceled so I haven't done any further work with Riak since late December.

Besides, I really appreciate creating autoconf and automake scripts, I never really had an occasion to learn how to write them and this is of a great help!

Back to studying your changes... ;)

I wasn't very careful to minimize the changes I made to your code, and I apologize for that. You are, of course, welcome to take as much or as little as you want. Thanks for your code.

David Hull

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