2

I am writing a C API for retrieving data from a hardware device. The data will be returned as a string with approximately 30 bytes of text per item. The problem is there may any number of items. Could just 10 or there could be 10,000. So the returned data could range from 10 x 30 = 300 to 10,000 x 30 = 300,000 bytes.

Initially, the function to get the data was like this:

int get_devicelist(char* devicelist);

So the onus is on the caller to know what they are doing and specify a big enough buffer. Trouble is software using API could be used in many end user applications so the programmer has no end user specific knowledge when they write code to use the API. The only option then is to specify a buffer (or allocate on heap) that is large enough to hold the largest possible amount of data returned.

So for that reason I am thinking about changing the API as here:

char* get_devicelist();
void free_devicelist(char* devlist);

eg in implementation:

char* get_devicelist() {
  size_t n = get_devices_size();
  char* devlist = malloc(n * 30);
  if(devlist)
     /* populate devlist */

  return devlist;
}

void free_devicelist(char* devlist) {
   free(devlist);
}

Or I just inform users to call free on devlist when no longer needed?

I would change the API code to get the number of devices and then allocate an appropriate amount of memory for devicelist. The only downside is that the user is then required to call free_devicelist to free the memory allocated.

Is the second API better? Is there an alternative way?

2
  • The current design is considered unsafe. This is a latent buffer overflow waiting to come over you and there is no way to prevent that, except always allocating an insane amount of memory.
    – JensG
    Oct 4, 2014 at 12:10
  • At what point in the process do you find out how many items there are? Can you reallocate, or must you always preallocate? You should make this clear in the question.
    – david.pfx
    Oct 4, 2014 at 13:59

3 Answers 3

2

The cleanest way to do this is to essentially expose an iterator which the caller can use to pull one fixed-size item out at a time.

In C, this can be done like the opendir/readdir/closedir POSIX interface. The first call allows the library to allocate some opaque iterator context, the second returns the current value from the sequence and advances the iterator, and the last destroys the iterator context.

So, your interface would look something like:

struct DEVICELIST; /* this is opaque to the caller */
struct Device {
  char description[30];
  /* or whatever data you want to return */
};

struct DEVICELIST *open_devicelist();
int read_device(struct DEVICELIST *, struct Device *current);
void close_devicelist(struct DEVICELIST *);

and your client code like:

struct Device dev;
struct DEVICELIST *dl = open_devicelist();
if (dl == NULL) return;
while (read_device(dl, &dev) == 0) {
    /* data has been copied into dev here, to use for whatever.
       Note it will be overwritten by the next device when we loop round.
    */
}
close_devicelist(dl);

If you want something less complex (I know you're just returning a string right now), your second suggestion is much better than the original.

Relying on the user to call your cleanup function is perfectly reasonable, and obviously no worse than the above.


(Note, if you look up the readdir call I mentioned, I'm actually using something more like readdir_r. The original non-reentrant version is a bit icky).

3
  • Interesting. I am looking at __dirstream (DIR) now and see it is a struct with an offset into the block (the block could be a linked list or an array I guess).
    – user619818
    Oct 4, 2014 at 18:01
  • I like the iterator approach, but offered an answer with a callback approach as well. What do you think?
    – supercat
    Aug 1, 2015 at 21:23
  • This is C, so the callback you suggested has to be a free function rather than a method. This also means you need to package up any local state needed in the callback into a struct, and (usually) pass it through as a void pointer. For synchronous processing it feels over-complicated (for async code it's worth the complexity, and in C++11 the barrier to use is much lower, since you can just pass a lambda).
    – Useless
    Aug 3, 2015 at 7:08
1

I'd recommend a look on your OSes APIs how to handle this. For the following example, I refer to the Windows API.

A number of functions with variable buffer outputs follow these rules:

int GetSomeString( char * pBuf, int nSize);

Return value:

  • If called with pBuf == NULL and nSize == 0, the function returns the required size of the buffer in characters, including the terminating NULL character
  • If called with pBuf != NULL, the nSize parameter should specifiy the size of *pBuf in characters, including the terminating NULL character. The function will fill the buffer up to nSize chars and return the number of characters written to the buffer

The user is responsible for allocating and freeing the memory.

As a user of the API, I personally find that quite handy. Next, because it is a common pattern, it will be easily understand.

1
  • I remember working with some of the Windows API functions yes. This approach is much better although I think best approach is from Useless - although more work.
    – user619818
    Oct 4, 2014 at 18:03
0

The iterator approach mentioned by Useless is pretty good, though it may be enhanced by having the "read more" function allow the caller to indicate how many items it is ready to accept. A limitation with that approach is that it may not be possible for a method to return a "snapshot" without tying up resources which could be left dangling if the caller fails to call a "close" method.

An alternative approach is to define a structure:

struct ThingProcessor {
  int (*process)(struct ThingProcessor*, struct Thing *, int n);
};

and then have a method:

void getThings(struct ThingProcessor *proc);

which will read groups of one or more items and, for each struct Thing someThings[n]; invoke proc->process(proc, Thing, n); [if the supplied method returns a negative value, getThings should exit early]. Provided that the supplied methods always return, the getThings() method will be able to ensure that any required resources get released.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.