Re: [PATCH] Add a `read' method for ports

[Neil Jerram]

Hi Ludovic,

2008/6/9 Ludovic Courtès <address@hidden>:
> Hi Neil,
>
> "Neil Jerram" <address@hidden> writes:
>
>> Sorry for not commenting before...  I've reviewed the history, and
>> started looking through the patches, and I'm feeling (possibly)
>> confused.  Am I correct in thinking that the "problem" here (which you
>> are aiming to solve) only affects unbuffered ports?
>
> Yes, such as sockets.

OK, understood, thanks.

>> If that's right, I'm not clear why the solution is involving so much
>> restructuring - can't it be done without introducing the new "read"
>> field?
>
> I don't think so.  Currently, when asking for N bytes at the application
> level, e.g., via `uniform-vector-read!' or `scm_c_read ()', we end up
> going through a series of `scm_fill_input ()'.  For unbuffered ports,
> this translates into N `scm_fill_input ()', and for unbuffered file
> ports, into N one-byte `read(2)' calls.

Yes, I see that now, and I agree that it is bad.

> IOW, the information about how many bytes are requested by the
> application isn't currently transferred to port implementations.  This
> is what the `read' method aims to solve.

I think there is a simpler solution, without introducing `read'.

One way of seeing this is that when a higher level function such as
scm_c_read or scm_uniform_vector_read requests a read of a specific
size from an `unbuffered' port, it is actually fine - and desirable -
for the port to use a buffer of that size.  When the user asks for
`unbuffered', they really only mean that they never want excess data
(i.e. more than they asked for) to be left in a buffer somewhere.

So, one way of solving this would be for an `unbuffered' port
temporarily to get a buffer of the right size, and then to use the
fill_input logic as it currently stands.  That would achieve your
objective of mapping onto a single N-byte read(2) call.

Then the question is just where the temporary buffer comes from.  Is
it provided by the caller, is it malloc'd and realloc'd and lives with
the port, or what?  (Note that if the buffer is allowed to persist
with the port, we can still get correct 'unbuffered' semantics by
reducing pt->read_buf_size back to 1.

I haven't thought through all the cases yet, but it feels as though
there should be a nice answer here.  For scm_c_read and
scm_uniform_vector_read, for example, the callers already have a
buffer, so all we need is a way to temporarily use that as the port's
read buffer.  Overall, it feels that this would involve less change
than your current patches, and it also feels more consistent (than the
read approach) with the existing API for explicitly adding a buffer to
a port.

(Are there actually _any_ cases where a caller does a read on a
possibly-unbuffered port, and either (i) has no idea of how much data
it is wanting, or (ii) doesn't already have a buffer that could be
used to do the reading?)

> The `read' method also has the advantage that it doesn't need to fiddle
> with `port->read_buf', unlike `fill_input' (but one still has to deal
> with it when instantiating a port).

But that advantage is also (in some cases) the disadvantage of having
to do an additional memcpy of the data.  I don't think it's too bad
for callers to be fiddling with read_buf - although we could
encapsulate that a bit more clearly.

What do you think?

      Neil