bug#30066: 'get-bytevector-some' returns only 1 byte from unbuffered ports

[Nala Ginrut]

hi Andy and Ludo!

What if developers enabled suspendable-ports and set the port to non-blocking?
For example, in the non-blocking asynchronous server, I registered
read/write waiter for suspendable-ports. And save
delimited-continuations then yield the current task.
In this situation, get-bytevector-n! will read n bytes with several
times yielding by the registered read-writer, from the caller's
perspective, get-bytevector-n! will return n bytes finally no matter
how many times it's yielded.
But how about the get-bytevector-some? Should it block just once and
return the first time read m bytes then return?

Thanks!


On Thu, Jan 11, 2018 at 12:32 AM, Andy Wingo <address@hidden> wrote:
> On Wed 10 Jan 2018 16:59, address@hidden (Ludovic Courtès) writes:
>
>> address@hidden (Ludovic Courtès) skribis:
>>
>>> As discussed on IRC, ‘get-bytevector-some’ returns only 1 byte from
>>> unbuffered ports:
>>
>> Here’s a tentative fix.  WDYT?
>
> Thanks!  Needs a little work though :)  Comments inline.
>
>> --- a/libguile/ports.h
>> +++ b/libguile/ports.h
>> @@ -69,6 +69,7 @@ SCM_INTERNAL SCM scm_i_port_weak_set;
>>  #define SCM_OPOUTPORTP(x) (SCM_OPPORTP (x) && SCM_OUTPUT_PORT_P (x))
>>  #define SCM_OPENP(x) (SCM_OPPORTP (x))
>>  #define SCM_CLOSEDP(x) (!SCM_OPENP (x))
>> +#define SCM_UNBUFFEREDP(x) (SCM_PORTP (x) && (SCM_CELL_WORD_0 (x) & 
>> SCM_BUF0))
>>  #define SCM_CLR_PORT_OPEN_FLAG(p) \
>>    SCM_SET_CELL_WORD_0 ((p), SCM_CELL_WORD_0 (p) & ~SCM_OPN)
>>  #ifdef BUILDING_LIBGUILE
>
> Please guard this under #ifdef BUILDING_LIBGUILE.
>
>> @@ -487,16 +487,33 @@ SCM_DEFINE (scm_get_bytevector_some, 
>> "get-bytevector-some", 1, 0, 0,
>>
>>    SCM_VALIDATE_BINARY_INPUT_PORT (1, port);
>>
>> -  buf = scm_fill_input (port, 0, &cur, &avail);
>> -  if (avail == 0)
>> +  if (SCM_UNBUFFEREDP (port))
>>      {
>> -      scm_port_buffer_set_has_eof_p (buf, SCM_BOOL_F);
>> -      return SCM_EOF_VAL;
>> +      size_t read;
>> +
>> +      bv = scm_c_make_bytevector (4096);
>> +      read = scm_i_read_bytes (port, bv, 0, SCM_BYTEVECTOR_LENGTH (bv));
>> +
>> +      if (read == 0)
>> +     return SCM_EOF_VAL;
>> +      else if (read < SCM_BYTEVECTOR_LENGTH (bv))
>> +     return scm_c_shrink_bytevector (bv, read);
>> +      else
>> +     return bv;
>>      }
>> +  else
>> +    {
>> +      buf = scm_fill_input (port, 0, &cur, &avail);
>> +      if (avail == 0)
>> +     {
>> +       scm_port_buffer_set_has_eof_p (buf, SCM_BOOL_F);
>> +       return SCM_EOF_VAL;
>> +     }
>>
>> -  bv = scm_c_make_bytevector (avail);
>> -  scm_port_buffer_take (buf, (scm_t_uint8 *) SCM_BYTEVECTOR_CONTENTS (bv),
>> -                        avail, cur, avail);
>> +      bv = scm_c_make_bytevector (avail);
>> +      scm_port_buffer_take (buf, (scm_t_uint8 *) SCM_BYTEVECTOR_CONTENTS 
>> (bv),
>> +                         avail, cur, avail);
>> +    }
>>
>>    return bv;
>>  }
>
> There are tabs in your code; would you mind doing only spaces?
>
> A port being unbuffered doesn't mean that it has no bytes in its
> buffer.  In particular, scm_unget_bytes may put bytes back into the
> buffer.  Or, peek-u8 might fill this buffer with one byte.
>
> Also, they port may have buffered write bytes (could be the port has
> write buffering but no read buffering).  In that case (pt->rw_random)
> you need to scm_flush().
>
> I suggest taking the buffered bytes from the read buffer, if any.  Then
> if the port is unbuffered, make a bytevector and call scm_i_read_bytes;
> otherwise do the scm_fill_input path that's there already.
>
> One more thing, if the port goes EOF, you need to
> scm_port_buffer_set_has_eof_p.
>
> Regards,
>
> Andy
>
>
>