[ccan] New module: iobuffer

[Joseph Adams]

On Fri, Jun 17, 2011 at 11:11 PM, Rusty Russell <rusty at rustcorp.com.au> wrote:
> My basic response: really good idea.  It's such a common thing to want
> to do; in an ideal world we'd be able to attach userspace backends to
> file descriptors and get this effect, but we can't.
>
> Since it's such a good idea, it's worth going over the API with a fine
> tooth comb.

Thanks!

> First, it's a classic multi-API situation: one for input, one for
> output, and then a part of each for implementing new input and output
> types.
>
> They almost certainly belong in the same module, but perhaps we should
> split the headers?  (I know ccanlint will complain if you don't have an
> iobuffer.h, but that can be fixed).

Maybe "iobuffer.h" should simply include the others.  I'd rather spare
users the tedium of having to include multiple headers for a single
library:

    #include <ccan/iobuffer/ibuffer.h>
    #include <ccan/iobuffer/obuffer.h>
    #include <ccan/iobuffer/drivers.h>

Ugh.

> The other question is, is it better to define a 'struct ibuffer_type'
> with the new, finish and fill operators inside *that*?  Then there's no
> need for an ibuffer_string_new() etc, just expose struct ibuffer_type
> ibuffer_string.

Good idea.  However, the new() and finish() callbacks have different
type signatures per driver.  For example:

    struct obuffer *obuffer_string_new(void);
    char *obuffer_string_finish(struct obuffer *ob, size_t *length_out);

    struct obuffer *obuffer_file_new(FILE *f);
    bool obuffer_file_finish(struct obuffer *ob);
    bool obuffer_file_finish_noclose(struct obuffer *ob);

When you finish an obuffer_string, you get a string out of it.  When
you create an obuffer_file, you need a FILE, and when you finish an
obuffer_file, you get an error indicator.

> Also, I think 's/ensure/need/' would tighten the API language a bit.

Makes sense.  However, the semantics for input versus output are a tad
different.  ibuffer_ensure may fail to obtain as many bytes as
requested, simply because those bytes might not be there
(end-of-file).  On the other hand, obuffer_ensure must always satisfy
the request (or else throw an exception, exit the program, etc.) so
callers don't have to keep checking over and over if the space is
available.

"ensure" suits obuffer's behavior better than ibuffer's, but I'm
inclined to agree that both ibuffer_ensure and obuffer_ensure should
be s/ensure/need/ .


The biggest hairy ends I see in my API are:

 * obuffer currently provides no mechanism for handling an
out-of-memory situation, other than expecting the user to use
exceptions (a TRY/CATCH module would be nice).
 * Writing a custom ibuffer or obuffer driver is tricky, since the
driver has to deal with buffering issues explicitly.  I could create
wrappers that take simple read and write callbacks, but that would
make things more complicated, as implementers would have to be aware
of a total of four interfaces.  I think helper functions are the way
to go.
 * The ensure (need) functions, and the callbacks they call, are sort
of redundant in their responsibilities.  While callers should
typically avoid calling (*refill) or (*flush) needlessly (using ensure
or manually checking if end - cur < need), the refill and flush
callbacks are also expected to handle being called when end - cur <
need reasonably well, rather than, say, doubling the buffer size every
time.
 * struct ibuffer and struct obuffer are really easy to mix up, as
well as tedious to type.  It helps that they're two separate types, so
the compiler will catch these mixups.  Perhaps they should be called
IN and OUT, or In and Out, instead of struct ibuffer and struct
obuffer?


Thanks!
Joey