Re: Request for Feedback: new BIO API

[Alessio Vanni]

(For some reason, I'm not receiving all the mails from this list.  Some
are arriving, some not.  Not sure what is going on, if there's an issue
in a configuration somewhere or simply a network problem; still, I had
to reply like this because I didn't receive anything and noticed the
reply only because I checked the web archive.  This mail is in reply to
Christian.)

> The GNUNET_BIO_write_create() API right now takes a buffer and size,
> which presumes that you know apriori how big the buffer will become
> (or at least have an upper bound). I think 'write_create()' should
> just take *no* arguments, use the GNUNET_Buffer-API internally to
> allocate as much space as needed, and then some new
> 'write_destroy_to_buffer()' function would *return* the GNUNET-buffer
> (or a void * and size_t) of the size that was required for all of the
> write operations.

The `write' side of the API has an upper bound because it was initially
designed to be used on already-allocated buffers, so something like
this:

char *buffer = GNUNET_new_array (some_size, char);
/* Maybe something going on here */
struct GNUNET_BIO_WriteHandle *wh = GNUNET_BIO_write_create (buffer, some_size);
/* Write operations here */

but admittedly using an internally-allocated buffer which is returned on
destroy is a better design. However, I think the `GNUNET_Buffer' API is
a bit underfeatured: to begin with, the function "reaping" the data
returns a 0-terminated string, which in this case is the wrong
representation.  It's probably trivial to implement a function returning
a `void *', though (I don't actually know how GNUNET_Buffers are
implemented as I have yet to read that part of the codebase.)

> The 'h' can't be in there, it should be provided only when the 'spec'
> is executed

The idea was to have something like this:

struct GNUNET_BIO_ReadHandle *h1 = GNUNET_BIO_read_open ("somefile");
struct GNUNET_BIO_ReadHandle *h2 = GNUNET_BIO_read_create (some_buffer, 
some_size);
struct GNUNET_BIO_ReadSpec r[] = {
 GNUNET_BIO_read_spec_string (h1, "string11", &some_string1, max_len1),
 GNUNET_BIO_read_spec_string (h1, "string12", &some_string2, max_len2),
 GNUNET_BIO_read_spec_string (h2, "string21", &some_string3, max_len3),
 GNUNET_BIO_read_spec_string (h2, "string22", &some_string4, max_len4),
 GNUNET_BIO_read_spec_end (),
};
GNUNET_BIO_read_spec_commit (r);

instead of having to declare multiple arrays for multiple operations.
Also I'm not sure what you mean by "and/or depend on the 'type' (thus
should be part of the internal knowledge of the callback!)".  `h' is the
Read/WriteHandle, so if it's part of the callback it would be the same
as what I implemented. Though I might've simply misunderstood.

As for the callback-based structure... that's probably a better design,
yes.  I'll work on a second version of the patch and submit it again
when it's ready.

Thanks,
A.V.