Re: Making custom binary input ports unbuffered

[Ludovic Courtès]

Mark H Weaver <address@hidden> skribis:

> Here are the same changes adapted for master, where we can put the
> new 'setvbuf' method where it belongs: in the PTOB.

Cool!

Wouldn’t it be easier to just merge stable-2.0 and modify from there?

> From 00ee913e2da658f30d9d8682edfbb9a63719c370 Mon Sep 17 00:00:00 2001
> From: Mark H Weaver <address@hidden>
> Date: Tue, 21 Jan 2014 01:57:31 -0500
> Subject: [PATCH 1/2] Prepare 'setvbuf' to support for non-file ports.
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> Based on a patch for Guile 2.0 by Ludovic Courtès.
>
> * libguile/fports.c (scm_fport_buffer_add): Rename to 'fport_setvbuf'.
>   (fport_setvbuf): Renamed from 'scm_fport_buffer_add'.  Change type of
>   'write_size' argument from 'int' to 'long'.
>   (scm_i_fdes_to_port): Adapt to renamed 'scm_fport_buffer_add'.
>   (scm_make_fptob): Set 'setvbuf' method to 'fport_setvbuf'.
>   (scm_setvbuf): Move to ports.c.
>
> * libguile/ports.c (scm_make_port_type): Initialize 'setvbuf' field.
>   (scm_set_port_setvbuf): New API function.
>   (scm_setvbuf): Moved from fports.c.  Accept any open port, and error
>   out when the ptob's 'setvbuf' field is NULL.  Remove explicit
>   'scm_gc_free' calls.  Call ptob's 'setvbuf' method instead of
>   'scm_fport_buffer_add'.
>
> * libguile/fports.h (scm_setbuf0): Remove prototype for non-existent
>   function.
>   (scm_setvbuf): Move prototype to ports.h.
>
> * libguile/ports.h (scm_t_ptob_descriptor): Add 'setvbuf' member.
>   (scm_set_port_setvbuf): Add prototype.
>   (scm_setvbuf): Move prototype here from fports.h.

Moving ‘setvbuf’ to ports.c also needs to be done in stable-2.0 (and
it’s worth making it in a separate patch, IMO.)

> * libguile/ports-internal.h (struct scm_port_internal): Change
>   'pending_eof' to a 1-bit unsigned char.  Add comment for 'alist'
>   member.
>
> * test-suite/tests/ports.test ("setvbuf")["closed port", "string port"]:
>   New tests.
>
> * doc/ref/api-io.texi (Port Implementation): Document new 'setvbuf'
>   member of ptob, and 'scm_set_port_setvbuf'.

I was wondering if the ‘setvbuf’ method should be exposed at this point,
because I wasn’t 100% sure of the API.  WDYT?

Perhaps it’s also better for a separate patch?

> address@hidden setvbuf
> +Create read and write buffers for the port with the specified sizes (a
> +size of 0 is for unbuffered ports, which should use the @code{shortbuf}
> +field; a size of -1 means to use the port's preferred buffer size).  It
> +doesn't need to be set unless you wish to support @code{setvbuf}.  Set
> +using
> +
> address@hidden void scm_set_port_setvbuf (scm_t_bits tc, void (*setvbuf) (SCM 
> port, long read_size, long write_size))
> address@hidden deftypefun

I guess the body was meant to go inside @deftypefun.  :-)

> From 6a3feb79b289410a62d2e0c8a70e0ea59d0cf8cd Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= <address@hidden>
> Date: Tue, 21 Jan 2014 02:28:35 -0500
> Subject: [PATCH 2/2] Custom binary input ports support 'setvbuf'.
>
> Modified-by: Mark H Weaver <address@hidden>
>
> * libguile/r6rs-ports.c (CBIP_BUFFER_SIZE): Adjust comment.  Set to 8KiB.
>   (SCM_SET_CBIP_BYTEVECTOR): New macro.
>   (cbip_setvbuf): New function.
>   (cbip_fill_input): Check whether PORT is buffered.  When unbuffered,
>   check whether BV can hold C_REQUESTED bytes, and allocate a new
>   bytevector if not; copy the data back from BV to c_port->read_pos.
>   Remove 'again' label, and don't loop there.
>   (initialize_custom_binary_input_ports): Set PTOB's 'setvbuf' method.
> * test-suite/tests/r6rs-ports.test ("7.2.7 Input Ports")["custom binary
>   input port unbuffered & 'port-position'", "custom binary input port
>   unbuffered & 'read!' calls", "custom binary input port, unbuffered
>   then buffered", "custom binary input port, buffered then unbuffered"]:
>   New tests.
> * doc/ref/api-io.texi (R6RS Binary Input): Document the buffering of
>   custom binary input ports, and link to 'setvbuf'.

OK.

Thanks for taking care of this!

Ludo’.