art_insert() should return a return code

[kellabyte]

This is the current definition of art_insert():

/**
 * Inserts a new value into the ART tree
 * @arg t The tree
 * @arg key The key
 * @arg key_len The length of the key
 * @arg value Opaque value.
 * @return NULL if the item was newly inserted, otherwise
 * the old value pointer is returned.
 */
void* art_insert(art_tree *t, const unsigned char *key, int key_len, void *value);

This is a really odd C return pattern. I don't like the idea that this could never fail. I'm sure there are some failure conditions and should return a return code. This function should tell me in the return code if something failed or if a key already exists.

Alternatively there could be a art_get_or_insert() function with a void** return argument for times you want to insert if it doesn't exist.

Also, why is there no set() function? What if I want to overwrite a value?

[armon]

@kellabyte Yep, that was definitely an API mistake. In retrospect, it should have returned the old value via an inout argument and returned an int status code, but ship has sailed... If you call art_insert on an existing key it will overwrite!

[kellabyte]

Ship has sailed? The API is frozen forever? I might have to second guess using this library if improvements are frozen for good like that.

[armon]

@kellabyte there is downstream code that would break if the function signature is changed, and I prefer to maintain backwards compatibility where possible. Another variant of art_insert could have a different type signature however.

[kellabyte]

Feels like to me you're lacking a release process. If you want to pin a piece of software to this API version create a branch with a version name and pin your down stream software to that release while you move master forward.

[armon]

@kellabyte that is fair. This project has effectively had a single release so I haven't invested time building a release project. It's not my intention to be difficult, but I don't have the time to maintain this full time. If you find it useful as is that's great, otherwise I hope you can either learn from it or fork and adapt it to your use case.