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

[Andy Wingo <wingo@igalia.com>]

On Wed 10 Jan 2018 16:59, ludo@gnu.org (Ludovic Courtès) writes:

> ludo@gnu.org (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