Best and safest API for a function which fills a buffer with variable-length data

Question

I have a function which receives a buffer and returns some data in the buffer. The data can be smaller than the buffer capacity.

Which is the best and safest API for this?

• int fn(void *buffer, size_t buffer_len): the size of the data written to buffer is returned by the function. Downside: the return value must also have a way to indicate that some error occurred (in-band error indicator).
• errno_t fn(void *buffer, size_t *buffer_len): in this case, buffer_len works both as input (the buffer capacity) and output (the data size). The function can return an error code. I think this is OK, but somewhat awkward.
• errno_t fn(void *buffer, size_t *data_len, size_t buffer_len): like the previous, but with input/output separated in two arguments. Also returns error code, but is also awkward due to too many arguments.

(Any other options?)

Answer 1

I think the best API for such case is to create a buffer specification and deal with it, like:

struct sbuf {
  void *data;
  size_t size, pos, length;
};

// void if malloc() errors do abort(). int otherwise, to report error
void sbuf_alloc(sbuf *sb, size_t minsize);
void sbuf_free(sbuf *sb);

int /* or errno_t, whatever */ fn(sbuf *sb);  

With such API, you'll avoid 1) long and cumbersome argument lists, 2) chance to mistake length for size and vice versa, and will gain instead understanding of object-like gist of your buffers.

Answer 2

A question we have pondered frequently.

1. Look at how similar situations are handled elsewhere in your API. Consistency is a very high priority and if there are established principles, stick to them.
2. The language matters. It is hard/impossible to return values in parameters in some languages (Java).
3. Consider using a struct or collection instead of a raw pointer. The byte array in Java carries its length with it.
4. I/O parameters are horrible to code with. Avoid them!
5. A return value should not be both a success indicator and a length, but if you break any rules this one is the least serious.

Overall my preference in C/C++ generally is to return bool success/failure, and provide a secondary function (GetLastError()) to get the real error code.

In the case of Read (only) my preference is to return the length read as return value of function, with a special value for error (-1). Not the nicest, but convenient.

If the data you are reading is known to be Ascii, then return a null-terminated string to give the length, and bool true/false for the function return.

[Regrettably this is all a matter of opinion, which makes this a question at risk of being closed.]

Answer 3

I'd support david's point of maintaining consistency, and since this seems to be C (and not C++), the required functionality is very much like the POSIX file read:
input buffer instead of a file.
output buffer for user, remains same.
number of bytes read or length, remains same.

So, I think sticking to POSIX is a good thing, and so the first option would be better, with a return of -1 for error (negative values for multiple errors is possible but appears clumsy):
int fn(void *buffer, size_t buffer_len)

Answer 4

Which approach is best depends upon when in the process the code which is filling the buffer will become aware of how big the buffer needs to be, and upon whether it would be possible to perform the operation partially, return to the caller, and then continue the operation later (after the caller either created a bigger buffer or done everything that will ever need to be done with the buffer's present contents, thus allowing the space to be reused). A nice general approach would be to pass a void** along with a function which given a void** and a requested size, will behave in a fashion roughly equivalent to:

void *resize(void **handle, size_t size)
{
  void *new_mem = realloc(*handle, size);
  if (!new_mem) return 0;
  *handle = new_mem;
  return new_mem;
}

but could use allocation mechanisms other than realloc to acquire memory. Using that approach would allow the function which is filling the buffer to expand it as its going along. Another approach is to define an abstract "state" structure, such that the usage pattern would be:

GRABBER_STATE *state = start_reading_data(...);
do {
  int n=get_items(state, buffer, buff_size));
  if (!n) break;
  process_n_items(buffer, n);
} while(1);
done_reading_data(state);

Here, it won't matter if the buffer is large enough to hold everything. What's important is that if there are items that don't get handled the first time through the loop, they can be handled on later passes.