On Wed, Apr 8, 2015 at 10:48 AM, Alan Conway <acon...@redhat.com> wrote:
> On Tue, 2015-04-07 at 17:57 -0400, Rafael Schloming wrote:
> > Maybe I'm not following something, but I don't see how passing around
> > allocation functions actually solves any ownership problems. I would
> think
> > the ownership problems come from pn_data_t holding onto a pointer
> > regardless of whether that pointer was gotten from malloc or from a
> > callback. I'm assuming you want to be able to do something like use a
> > pn_data_t that is owned by one thread to encode into a buffer and pass
> that
> > buffer over to another thread. How does passing in a bunch of allocators
> > help with this? If the pn_data_t holds a pointer to whatever those
> > allocators return, then aren't you going to have ownership issues no
> matter
> > what?
> >
> > To answer Bozzo's original question, I think that it's good to keep the
> > encoder/decoder decoupled from the buffer for a number of reasons. In
> > addition to the ownership issues that Alan points out, the encoded data
> may
> > have a different lifecycle from the pn_data_t that created it for non
> > thread related reasons, or you may simply want to encode directly into a
> > frame buffer and avoid an extra copy.
> >
> > If the goal here is simply to provide a convenient way to avoid having to
> > repeat the resizing loop then I would suggest simply providing a
> > convenience API that accepts a growable buffer of some sort. This
> provides
> > both convenience and avoids ownership issues with holding pointers. I'm
> > thinking something along the lines of:
> >
> > pn_data_t data = ...;
> > pn_string/buffer_t buf = ...;
> > pn_data_encode_conveniently(data, buf); // obviously needs a better
> name
> >
> > It is perhaps not *quite* as convenient in the minimal case as pn_data_t
> > holding the buffer internally, but it is an improvement in general and
> > probably simpler than having to mess with function pointers in the case
> > where the buffer's lifecycle is independent from the pn_data_t. (Not
> that I
> > really understand how that would work anyways.)
>
> The issue is that we need buffer growth AND control over allocation.
> pn_buffer_t forces use of malloc/free/realloc. That won't help if you're
> trying to get the data into a buffer allocated by Go for example. I
> agree a growable buffer type is a nicer API than raw function pointers,
> but the buffer type itself would need to use function pointers so we can
> replace the functions used for alloc/free/realloc.
>
I'm skeptical that passing around a set of function pointers whether
directly to pn_data_t or to pn_buffer_t is actually fewer lines of code
than simply writing the analogous convenience encoding function for that
language. For python it would likely be fewer lines of code to simply
define something like this:
ssize_t pn_data_encode2bytearray(pn_data_t *data, PyObject *bytearray) {
ssize_t size = pn_data_encode(data,
PyByteArray_AsString(bytearray), PyByteArray_Size(bytearray));
if (size < 0) { return size; }
if (size > PyByteArray_Size(bytearray)) {
PyByteArray_Resize(bytearray, size);
return pn_data_encode(data, PyByteArray_AsString(bytearray),
PyByteArray_Size(bytearray));
}
}
This would also be more flexible since you could pass in any bytearray
object from python, reuse it as much as you like, and have full control
over the lifecycle of that object, whereas I suspect given the signature of
the function pointers you are defining you would need to choose some sort
of predefined allocation strategy for creating whatever go object you are
encoding into. This sounds to me like it is both less convenient and less
flexible, but its entirely possible I'm just not understanding how you
intend to use this. Perhaps it would clarify things if you could post an
actual example usage of how you imagine using this that hopefully
illustrates why it is fewer lines of code than the alternatives?
--Rafael
