Proposal

[ethanfrey]

I pulled together all learnings from go-cosmwam work and make a simple example of
how to call code with various behaviors.

I'd love feedback on how to develop this for even cleaner APIs.

[matklad]

Using errno-style thread local for a modern C-library is not the best approach.

I think what you are supposed to do in modern C is using an out parameter for error, like this:

struct ErrorInfo {
    char* message; // statically allcoated message, which doesn't need free
}

// 0 means success, != 0 -- error
int frobnicate(int input1, int input2, my_struct* output, ErrorInfo* error);

The call site would look like this

struct ErrorInfo error; // reused across many calls;

struct MyStruct res;
if !frobnicate(92, 62, &res, &error) {
    fprintf(2, "error: %s", error.message);
    return;
}

[matklad]

I am pretty sure I am not thinking up, and that this is indeed written down in some blog post, but, alas, googling "modern C error management" doesn't help :(

[ethanfrey]

Thanks for the review.

On this particular point, I didn't really choose to use errno, this is what cgo recommends (and this lib is designed to be consumed with go):

Any C function (even void functions) may be called in a multiple assignment context to retrieve both the return value (if any) and the C errno variable as an error (use _ to skip the result value if the function returns void). For example:
https://golang.org/cmd/cgo/

The use of thread_local and RefCell was something I copied from another lib in order to get a string error message (not just a number) and seems to make having one static value less dangerous. But yes... I could definitely envision a better solution.

In particular, you suggest using the return value instead of errno (0 success, other failure), and then returning any result and error as pointer arguments. It seems to make uglier function definitions on one hand. On the other, it does remove some "magic" code and make the actual information passing explicity (not "put this error in a box for later and hope someone picks it up").

[matklad]

Oh, if cgo requires errno for convenient API on the Go side, than our hands are tied. It maybe makes scene to add a comment like "we use errno because cgo has special integration with it".

For getting messages out of errors, I would try, if possible, to restrict them to a set of static strings, so that the caller doesn't have to worry about allocation/deallocation. The API then may look like this:

pub extern "c" fn get_last_error() -> *const u8 { // returns static C-string or null, by decoding errno value
}

[ethanfrey]

Interesting point about static strings.
I just declare a whole set of them as consts in one file and reference them?

I could even use the &error approach to return the string in the same call, even with errno (that would just change the meaning of the return value)

[ethanfrey]

@matklad I cleaned up the memory handling. Thank you for the advice, I do think it looks cleaner now.

I will reflect a bit more on the errors... I would like to accept Result<T, Error> and then pass that error message over the FFI to allow normal error handling in the rust code inside. However, that doesn't allow the static string lookup table (which obviously has nice benefits).

I think of passing in error: *mut Buffer as an extra arg everywhere and setting it there on error, removing get_last_error. Still using errno for now. This would require the caller to free the memory after read (as we do when getting a Buffer result), but would remove the confusion of the error.rs file, statics and thread_local.

These are just thoughts. Open for feedback. I will work on it tonight or tomorrow.

[ethanfrey]

I cleaned up the error returns somewhat.

Still use errno to signal there was an error, but then write into a provided buffer (Option<&mut Buffer>) with the message, to avoid the odd callback and thread-local RefCell