* Making custom binary input ports unbuffered
@ 2014-01-14 23:00 Ludovic Courtès
2014-01-15 5:43 ` Mark H Weaver
0 siblings, 1 reply; 11+ messages in thread
From: Ludovic Courtès @ 2014-01-14 23:00 UTC (permalink / raw)
To: guile-devel; +Cc: Andy Wingo, Mark H Weaver
[-- Attachment #1: Type: text/plain, Size: 1770 bytes --]
Hi!
As discussed on IRC, our R6 custom binary input ports (CBIPs) are
currently buffered. The buffer is hard-coded and setvbuf doesn’t work
on non-file ports.
Having a buffer can be problematic for several reasons.
1. The user’s ‘get-position’ will always point past what the port’s
user sees.
This could be worked around by subtracting read_pos to what
‘get-position’ returns, but that feels wrong: conceptually, the
port’s position is something under the CBIP implementor’s control.
(I wonder how fopencookie/fseek deal with this.)
2. Some applications want no buffering.
My use case was that I read from a byte stream, and at some point I
want to compute a hash over a delimited part of that stream. To do
that, I intuitively wanted to have a CBIP that wraps some other
input port, and do the hash computation in that CBIP. But that
only works if we can guarantee that the CBIP doesn’t read more than
what was actually asked.
Same for the delimited port in (web response).
The patch below makes CBIPs unbuffered (see the tests for the properties
it gives.) It works thanks to the optimization in ‘scm_c_read’ for
unbuffered binary ports.
This is going to be a performance hit for applications that read things
byte by byte, *or* via textual procedures (‘scm_getc’, ‘get-string’,
etc.) But the assumption is that people rather use ‘get-bytevector-n’
(or similar) to get a chunk of data.
However! There are places in (web ...) where CBIPs are used for mixed
binary/textual input. When that happens, all the accesses end up being
unbuffered, which really sucks.
So, what do we do? :-)
Thanks,
Ludo’.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: the patch --]
[-- Type: text/x-patch, Size: 9200 bytes --]
From ef60588d6e76d6ad0ae09197043c6d7371beb1b7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= <ludo@gnu.org>
Date: Tue, 14 Jan 2014 21:54:51 +0100
Subject: [PATCH] Make custom binary input ports unbuffered.
* libguile/r6rs-ports.c (make_cbip): Leave read_{pos,end,buf_size}
unchanged and call 'scm_port_non_buffer'.
(cbip_fill_input): Change to use PORT's associated bytevector as an
intermediate copy passed to READ_PROC and then copied back into PORT's
own buffer. Reallocate a new bytevector when it's smaller than the
current 'read_buf_size'. Don't loop back to 'again' label and remove
it.
* test-suite/tests/r6rs-ports.test ("7.2.7 Input Ports")["custom binary
input & 'port-position'", "custom binary input & 'read!' calls"]: New
tests.
---
libguile/r6rs-ports.c | 77 ++++++++++++++++++++++++++--------------
test-suite/tests/r6rs-ports.test | 60 +++++++++++++++++++++++++++++++
2 files changed, 110 insertions(+), 27 deletions(-)
diff --git a/libguile/r6rs-ports.c b/libguile/r6rs-ports.c
index 790c24c..f2a654e 100644
--- a/libguile/r6rs-ports.c
+++ b/libguile/r6rs-ports.c
@@ -280,13 +280,17 @@ cbp_close (SCM port)
static scm_t_bits custom_binary_input_port_type = 0;
-/* Size of the buffer embedded in custom binary input ports. */
+/* Initial size of the buffer embedded in custom binary input ports. */
#define CBIP_BUFFER_SIZE 4096
/* Return the bytevector associated with PORT. */
#define SCM_CBIP_BYTEVECTOR(_port) \
SCM_SIMPLE_VECTOR_REF (SCM_PACK (SCM_STREAM (_port)), 4)
+/* Set BV as the bytevector associated with PORT. */
+#define SCM_SET_CBIP_BYTEVECTOR(_port, _bv) \
+ SCM_SIMPLE_VECTOR_SET (SCM_PACK (SCM_STREAM (_port)), 4, (_bv))
+
/* Return the various procedures of PORT. */
#define SCM_CBIP_READ_PROC(_port) \
SCM_SIMPLE_VECTOR_REF (SCM_PACK (SCM_STREAM (_port)), 0)
@@ -297,15 +301,11 @@ make_cbip (SCM read_proc, SCM get_position_proc,
SCM set_position_proc, SCM close_proc)
{
SCM port, bv, method_vector;
- char *c_bv;
- unsigned c_len;
scm_t_port *c_port;
const unsigned long mode_bits = SCM_OPN | SCM_RDNG;
- /* Use a bytevector as the underlying buffer. */
- c_len = CBIP_BUFFER_SIZE;
- bv = scm_c_make_bytevector (c_len);
- c_bv = (char *) SCM_BYTEVECTOR_CONTENTS (bv);
+ /* Pre-allocate a bytevector to be passed to the 'read!' method. */
+ bv = scm_c_make_bytevector (CBIP_BUFFER_SIZE);
/* Store the various methods and bytevector in a vector. */
method_vector = scm_c_make_vector (5, SCM_BOOL_F);
@@ -326,10 +326,13 @@ make_cbip (SCM read_proc, SCM get_position_proc,
/* Attach it the method vector. */
SCM_SETSTREAM (port, SCM_UNPACK (method_vector));
- /* Have the port directly access the buffer (bytevector). */
- c_port->read_pos = c_port->read_buf = (unsigned char *) c_bv;
- c_port->read_end = (unsigned char *) c_bv;
- c_port->read_buf_size = c_len;
+ /* Make it unbuffered. This is necessary to guarantee that (1) users can
+ actually implement GET_POSITION_PROC correctly, and that (2) each
+ 'get-bytevector-*' call has exactly one corresponding READ_PROC call.
+ The latter is necessary in some applications, typically when wrapping
+ another port where we don't want to consume more than what was
+ actually asked for. */
+ scm_port_non_buffer (c_port);
/* Mark PORT as open, readable and unbuffered (hmm, how elegant...). */
SCM_SET_CELL_TYPE (port, custom_binary_input_port_type | mode_bits);
@@ -346,34 +349,54 @@ cbip_fill_input (SCM port)
int result;
scm_t_port *c_port = SCM_PTAB_ENTRY (port);
- again:
if (c_port->read_pos >= c_port->read_end)
{
/* Invoke the user's `read!' procedure. */
- unsigned c_octets;
+ size_t c_octets, c_requested;
SCM bv, read_proc, octets;
- /* Use the bytevector associated with PORT as the buffer passed to the
+ read_proc = SCM_CBIP_READ_PROC (port);
+
+ /* Attempt to pass the bytevector associated with PORT to the
`read!' procedure, thereby avoiding additional allocations. */
bv = SCM_CBIP_BYTEVECTOR (port);
- read_proc = SCM_CBIP_READ_PROC (port);
- /* The assumption here is that C_PORT's internal buffer wasn't changed
- behind our back. */
- assert (c_port->read_buf ==
- (unsigned char *) SCM_BYTEVECTOR_CONTENTS (bv));
- assert ((unsigned) c_port->read_buf_size
- == SCM_BYTEVECTOR_LENGTH (bv));
+ /* When called via the 'get-bytevector-*' procedures, and thus via
+ 'scm_c_read', we are passed the caller-provided buffer, so we need
+ to check its size. */
+ c_requested = c_port->read_buf_size;
+
+ if (SCM_BYTEVECTOR_LENGTH (bv) < c_requested)
+ {
+ /* Bad luck: we have to make another allocation. Save that
+ bytevector for later reuse, in the hope that the application
+ has regular access patterns. */
+ bv = scm_c_make_bytevector (c_requested);
+ SCM_SET_CBIP_BYTEVECTOR (port, bv);
+ }
+
+ /* READ_PROC must always be called with a strictly positive number of
+ bytes to read; otherwise it is forced to return 0, which is used
+ to indicate EOF. */
+ if (SCM_LIKELY (c_requested > 0))
+ {
+ octets = scm_call_3 (read_proc, bv, SCM_INUM0,
+ scm_from_size_t (c_requested));
+ c_octets = scm_to_size_t (octets);
+ }
+ else
+ c_octets = 0;
- octets = scm_call_3 (read_proc, bv, SCM_INUM0,
- SCM_I_MAKINUM (CBIP_BUFFER_SIZE));
- c_octets = scm_to_uint (octets);
+ if (SCM_UNLIKELY (c_octets > c_requested))
+ scm_out_of_range (FUNC_NAME, octets);
- c_port->read_pos = (unsigned char *) SCM_BYTEVECTOR_CONTENTS (bv);
+ /* Copy the data back to the original buffer. */
+ memcpy ((char *) c_port->read_pos, SCM_BYTEVECTOR_CONTENTS (bv),
+ c_octets);
c_port->read_end = (unsigned char *) c_port->read_pos + c_octets;
- if (c_octets > 0)
- goto again;
+ if (c_octets != 0 || c_requested == 0)
+ result = (int) *c_port->read_pos;
else
result = EOF;
}
diff --git a/test-suite/tests/r6rs-ports.test b/test-suite/tests/r6rs-ports.test
index eaae29f..41b46b1 100644
--- a/test-suite/tests/r6rs-ports.test
+++ b/test-suite/tests/r6rs-ports.test
@@ -447,6 +447,66 @@ not `set-port-position!'"
(u8-list->bytevector
(map char->integer (string->list "Port!")))))))
+ (pass-if-equal "custom binary input & 'port-position'"
+ '(0 2 5 11)
+ ;; Check that the value returned by 'port-position' is correct, and
+ ;; that each 'port-position' call leads one call to the
+ ;; 'get-position' method.
+ (let* ((str "Hello Port!")
+ (output (make-bytevector (string-length str)))
+ (source (with-fluids ((%default-port-encoding "UTF-8"))
+ (open-string-input-port str)))
+ (read! (lambda (bv start count)
+ (let ((r (get-bytevector-n! source bv start count)))
+ (if (eof-object? r)
+ 0
+ r))))
+ (pos '())
+ (get-pos (lambda ()
+ (let ((p (port-position source)))
+ (set! pos (cons p pos))
+ p)))
+ (port (make-custom-binary-input-port "the port" read!
+ get-pos #f #f)))
+
+ (and (= 0 (port-position port))
+ (begin
+ (get-bytevector-n! port output 0 2)
+ (= 2 (port-position port)))
+ (begin
+ (get-bytevector-n! port output 2 3)
+ (= 5 (port-position port)))
+ (let ((bv (string->utf8 (get-string-all port))))
+ (bytevector-copy! bv 0 output 5 (bytevector-length bv))
+ (= (string-length str) (port-position port)))
+ (bytevector=? output (string->utf8 str))
+ (reverse pos))))
+
+ (pass-if-equal "custom binary input & 'read!' calls"
+ `((2 "He") (3 "llo") (42 " Port!"))
+ (let* ((str "Hello Port!")
+ (source (with-fluids ((%default-port-encoding "UTF-8"))
+ (open-string-input-port str)))
+ (reads '())
+ (read! (lambda (bv start count)
+ (set! reads (cons count reads))
+ (let ((r (get-bytevector-n! source bv start count)))
+ (if (eof-object? r)
+ 0
+ r))))
+ (port (make-custom-binary-input-port "the port" read!
+ #f #f #f)))
+
+ (let ((ret (list (get-bytevector-n port 2)
+ (get-bytevector-n port 3)
+ (get-bytevector-n port 42))))
+ (zip (reverse reads)
+ (map (lambda (obj)
+ (if (bytevector? obj)
+ (utf8->string obj)
+ obj))
+ ret)))))
+
(pass-if "custom binary input port `close-proc' is called"
(let* ((closed? #f)
(read! (lambda (bv start count) 0))
--
1.8.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: Making custom binary input ports unbuffered
2014-01-14 23:00 Making custom binary input ports unbuffered Ludovic Courtès
@ 2014-01-15 5:43 ` Mark H Weaver
2014-01-15 11:48 ` Ludovic Courtès
2014-01-15 22:51 ` Ludovic Courtès
0 siblings, 2 replies; 11+ messages in thread
From: Mark H Weaver @ 2014-01-15 5:43 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: Andy Wingo, guile-devel
Hi Ludovic,
ludo@gnu.org (Ludovic Courtès) writes:
> As discussed on IRC, our R6 custom binary input ports (CBIPs) are
> currently buffered. The buffer is hard-coded and setvbuf doesn’t work
> on non-file ports.
[...]
> The patch below makes CBIPs unbuffered (see the tests for the properties
> it gives.) It works thanks to the optimization in ‘scm_c_read’ for
> unbuffered binary ports.
When you say "It works thanks to the optimization in 'scm_c_read'", do
you mean that something will be broken if another method of reading is
used?
I think it's important that everything that currently works keeps on
working, even if less efficiently.
> This is going to be a performance hit for applications that read things
> byte by byte, *or* via textual procedures (‘scm_getc’, ‘get-string’,
> etc.) But the assumption is that people rather use ‘get-bytevector-n’
> (or similar) to get a chunk of data.
>
> However! There are places in (web ...) where CBIPs are used for mixed
> binary/textual input. When that happens, all the accesses end up being
> unbuffered, which really sucks.
>
> So, what do we do? :-)
While I agree that CBIPs should be unbuffered by default, I think it's
important to have a way to enable buffering, even if it's not portable.
Also, I think we should make R6RS custom binary _output_ ports
unbuffered by default as well, and we should have a way to enable
buffering on them.
Thanks for looking into this!
Mark
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Making custom binary input ports unbuffered
2014-01-15 5:43 ` Mark H Weaver
@ 2014-01-15 11:48 ` Ludovic Courtès
2014-01-15 22:51 ` Ludovic Courtès
1 sibling, 0 replies; 11+ messages in thread
From: Ludovic Courtès @ 2014-01-15 11:48 UTC (permalink / raw)
To: Mark H Weaver; +Cc: Andy Wingo, guile-devel
Mark H Weaver <mhw@netris.org> skribis:
> ludo@gnu.org (Ludovic Courtès) writes:
>
>> As discussed on IRC, our R6 custom binary input ports (CBIPs) are
>> currently buffered. The buffer is hard-coded and setvbuf doesn’t work
>> on non-file ports.
>
> [...]
>
>> The patch below makes CBIPs unbuffered (see the tests for the properties
>> it gives.) It works thanks to the optimization in ‘scm_c_read’ for
>> unbuffered binary ports.
>
> When you say "It works thanks to the optimization in 'scm_c_read'", do
> you mean that something will be broken if another method of reading is
> used?
No; I mean that without that optimization ‘read!’ would be called for
one byte at a time.
>> This is going to be a performance hit for applications that read things
>> byte by byte, *or* via textual procedures (‘scm_getc’, ‘get-string’,
>> etc.) But the assumption is that people rather use ‘get-bytevector-n’
>> (or similar) to get a chunk of data.
>>
>> However! There are places in (web ...) where CBIPs are used for mixed
>> binary/textual input. When that happens, all the accesses end up being
>> unbuffered, which really sucks.
>>
>> So, what do we do? :-)
>
> While I agree that CBIPs should be unbuffered by default, I think it's
> important to have a way to enable buffering, even if it's not portable.
Yeah, that’s probably the right thing to do.
I’m a bit afraid of the consequences of allowing non-file ports for
‘setvbuf’, but I’ll experiment and see if it causes any problems.
Thanks for your feedback!
Ludo’.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Making custom binary input ports unbuffered
2014-01-15 5:43 ` Mark H Weaver
2014-01-15 11:48 ` Ludovic Courtès
@ 2014-01-15 22:51 ` Ludovic Courtès
2014-01-16 0:15 ` Mark H Weaver
1 sibling, 1 reply; 11+ messages in thread
From: Ludovic Courtès @ 2014-01-15 22:51 UTC (permalink / raw)
To: Mark H Weaver; +Cc: Andy Wingo, guile-devel
[-- Attachment #1: Type: text/plain, Size: 848 bytes --]
Mark H Weaver <mhw@netris.org> skribis:
> While I agree that CBIPs should be unbuffered by default, I think it's
> important to have a way to enable buffering, even if it's not portable.
So I started with the patch below, which allows us to specify which
ports (really: which port types) support ‘setvbuf’. This is
conservative so that ports that do not explicitly claim to support it
won’t break.
Then I started modifying the CBIP. The problem is that the CBIP has a
bytevector stored in its SCM_STREAM, whose contents are normally stored
in ‘read_buf’. So it needs to know if a new buffer is installed.
Perhaps that, rather than a ‘supports_setvbuf’ bit, we need a pointer to
the port’s ‘setvbuf’ method? For file ports it would be
‘scm_fport_buffer_add’, for instance.
Thoughts?
Ludo’.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: the patch --]
[-- Type: text/x-patch, Size: 6368 bytes --]
From 0b9ddbe6085aff2224203a8f125417ccadc2463b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= <ludo@gnu.org>
Date: Wed, 15 Jan 2014 17:38:44 +0100
Subject: [PATCH] Prepare 'setvbuf' to support for non-file ports.
* libguile/ports-internal.h (struct scm_port_internal): Add
'supports_setvbuf' field. Change 'pending_eof' to a 1-bit unsigned
char.
* libguile/ports.c (scm_new_port_table_entry): Clear
'pti->supports_setvbuf'.
* libguile/fports.c (scm_setvbuf): Accept any open port, and error out
when PORT's 'supports_setvbuf' field is cleared. Remove explicit
'scm_gc_free' calls.
(scm_i_fdes_to_port): Set PORT's 'supports_setvbuf' flag.
* test-suite/tests/ports.test ("setvbuf")["closed port", "string port"]:
New tests.
---
libguile/fports.c | 24 +++++++++++++++---------
libguile/ports-internal.h | 11 +++++++++--
libguile/ports.c | 8 +++++++-
test-suite/tests/ports.test | 14 +++++++++++++-
4 files changed, 44 insertions(+), 13 deletions(-)
diff --git a/libguile/fports.c b/libguile/fports.c
index 70732e5..0246512 100644
--- a/libguile/fports.c
+++ b/libguile/fports.c
@@ -58,6 +58,7 @@
#include "libguile/hashtab.h"
#include "libguile/fports.h"
+#include "libguile/ports-internal.h"
#if SIZEOF_OFF_T == SIZEOF_INT
#define OFF_T_MAX INT_MAX
@@ -158,7 +159,11 @@ SCM_DEFINE (scm_setvbuf, "setvbuf", 2, 1, 0,
port = SCM_COERCE_OUTPORT (port);
- SCM_VALIDATE_OPFPORT (1,port);
+ SCM_VALIDATE_OPENPORT (1, port);
+ if (!SCM_PORT_GET_INTERNAL (port)->supports_setvbuf)
+ scm_wrong_type_arg_msg (FUNC_NAME, 1, port,
+ "port that supports 'setvbuf'");
+
cmode = scm_to_int (mode);
if (cmode != _IONBF && cmode != _IOFBF && cmode != _IOLBF)
scm_out_of_range (FUNC_NAME, mode);
@@ -169,9 +174,8 @@ SCM_DEFINE (scm_setvbuf, "setvbuf", 2, 1, 0,
cmode = _IOFBF;
}
else
- {
- SCM_SET_CELL_WORD_0 (port, SCM_CELL_WORD_0 (port) & ~(scm_t_bits)SCM_BUFLINE);
- }
+ SCM_SET_CELL_WORD_0 (port,
+ SCM_CELL_WORD_0 (port) & ~(scm_t_bits) SCM_BUFLINE);
if (SCM_UNBNDP (size))
{
@@ -216,10 +220,6 @@ SCM_DEFINE (scm_setvbuf, "setvbuf", 2, 1, 0,
pt->read_end = pt->saved_read_end;
pt->read_buf_size = pt->saved_read_buf_size;
}
- if (pt->read_buf != &pt->shortbuf)
- scm_gc_free (pt->read_buf, pt->read_buf_size, "port buffer");
- if (pt->write_buf != &pt->shortbuf)
- scm_gc_free (pt->write_buf, pt->write_buf_size, "port buffer");
scm_fport_buffer_add (port, csize, csize);
@@ -542,6 +542,7 @@ scm_i_fdes_to_port (int fdes, long mode_bits, SCM name)
{
SCM port;
scm_t_port *pt;
+ scm_t_port_internal *pti;
/* Test that fdes is valid. */
#ifdef F_GETFL
@@ -567,7 +568,12 @@ scm_i_fdes_to_port (int fdes, long mode_bits, SCM name)
port = scm_new_port_table_entry (scm_tc16_fport);
SCM_SET_CELL_TYPE(port, scm_tc16_fport | mode_bits);
- pt = SCM_PTAB_ENTRY(port);
+ pt = SCM_PTAB_ENTRY (port);
+
+ /* File ports support 'setvbuf'. */
+ pti = SCM_PORT_GET_INTERNAL (port);
+ pti->supports_setvbuf = 1;
+
{
scm_t_fport *fp
= (scm_t_fport *) scm_gc_malloc_pointerless (sizeof (scm_t_fport),
diff --git a/libguile/ports-internal.h b/libguile/ports-internal.h
index 8a3a00b..2243744 100644
--- a/libguile/ports-internal.h
+++ b/libguile/ports-internal.h
@@ -1,7 +1,7 @@
/*
* ports-internal.h - internal-only declarations for ports.
*
- * Copyright (C) 2013 Free Software Foundation, Inc.
+ * Copyright (C) 2013, 2014 Free Software Foundation, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public License
@@ -50,7 +50,14 @@ struct scm_port_internal
unsigned at_stream_start_for_bom_write : 1;
scm_t_port_encoding_mode encoding_mode;
scm_t_iconv_descriptors *iconv_descriptors;
- int pending_eof;
+ unsigned char pending_eof: 1;
+
+ /* Whether this port supports 'setvbuf'. If true, that means that
+ 'setvbuf' is free to fiddle with its internal buffers.
+ XXX: Make this a property of the 'scm_t_ptob_descriptor'. */
+ unsigned char supports_setvbuf: 1;
+
+ /* Key-value properties. */
SCM alist;
};
diff --git a/libguile/ports.c b/libguile/ports.c
index 4516160..dbd22b9 100644
--- a/libguile/ports.c
+++ b/libguile/ports.c
@@ -1,5 +1,5 @@
/* Copyright (C) 1995, 1996, 1997, 1998, 1999, 2000, 2001, 2003, 2004, 2006,
- * 2007, 2008, 2009, 2010, 2011, 2012, 2013 Free Software Foundation, Inc.
+ * 2007, 2008, 2009, 2010, 2011, 2012, 2013, 2014 Free Software Foundation, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public License
@@ -677,6 +677,12 @@ scm_new_port_table_entry (scm_t_bits tag)
pti->pending_eof = 0;
pti->alist = SCM_EOL;
+ /* Until Guile 2.0.9 included, 'setvbuf' would only work on file
+ ports. Now all port types can be supported, but it's not clear
+ that port types out in wild accept having someone else fiddle with
+ their buffer. Thus, conservatively turn it off by default. */
+ pti->supports_setvbuf = 0;
+
SCM_SET_CELL_TYPE (z, tag);
SCM_SETPTAB_ENTRY (z, entry);
diff --git a/test-suite/tests/ports.test b/test-suite/tests/ports.test
index 9b1c6c0..c2f4480 100644
--- a/test-suite/tests/ports.test
+++ b/test-suite/tests/ports.test
@@ -2,7 +2,7 @@
;;;; Jim Blandy <jimb@red-bean.com> --- May 1999
;;;;
;;;; Copyright (C) 1999, 2001, 2004, 2006, 2007, 2009, 2010,
-;;;; 2011, 2012, 2013 Free Software Foundation, Inc.
+;;;; 2011, 2012, 2013, 2014 Free Software Foundation, Inc.
;;;;
;;;; This library is free software; you can redistribute it and/or
;;;; modify it under the terms of the GNU Lesser General Public
@@ -1499,6 +1499,18 @@
(with-test-prefix "setvbuf"
+ (pass-if-exception "closed port"
+ exception:wrong-type-arg
+ (let ((port (open-input-file "/dev/null")))
+ (close-port port)
+ (setvbuf port _IOFBF)))
+
+ (pass-if-exception "string port"
+ exception:wrong-type-arg
+ (let ((port (open-input-string "Hey!")))
+ (close-port port)
+ (setvbuf port _IOFBF)))
+
(pass-if "line/column number preserved"
;; In Guile 2.0.5, `setvbuf' would erroneously decrease the port's
;; line and/or column number.
--
1.8.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: Making custom binary input ports unbuffered
2014-01-15 22:51 ` Ludovic Courtès
@ 2014-01-16 0:15 ` Mark H Weaver
2014-01-16 23:00 ` Ludovic Courtès
0 siblings, 1 reply; 11+ messages in thread
From: Mark H Weaver @ 2014-01-16 0:15 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: Andy Wingo, guile-devel
ludo@gnu.org (Ludovic Courtès) writes:
> Mark H Weaver <mhw@netris.org> skribis:
>
>> While I agree that CBIPs should be unbuffered by default, I think it's
>> important to have a way to enable buffering, even if it's not portable.
>
> So I started with the patch below, which allows us to specify which
> ports (really: which port types) support ‘setvbuf’. This is
> conservative so that ports that do not explicitly claim to support it
> won’t break.
>
> Then I started modifying the CBIP. The problem is that the CBIP has a
> bytevector stored in its SCM_STREAM, whose contents are normally stored
> in ‘read_buf’. So it needs to know if a new buffer is installed.
>
> Perhaps that, rather than a ‘supports_setvbuf’ bit, we need a pointer to
> the port’s ‘setvbuf’ method? For file ports it would be
> ‘scm_fport_buffer_add’, for instance.
Yes, it sounds like we'll need custom 'setvbuf' methods.
In master, maybe we can move it to scm_t_ptob_descriptor.
Thanks,
Mark
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Making custom binary input ports unbuffered
2014-01-16 0:15 ` Mark H Weaver
@ 2014-01-16 23:00 ` Ludovic Courtès
2014-01-18 21:57 ` Ludovic Courtès
2014-01-21 7:41 ` Mark H Weaver
0 siblings, 2 replies; 11+ messages in thread
From: Ludovic Courtès @ 2014-01-16 23:00 UTC (permalink / raw)
To: Mark H Weaver; +Cc: Andy Wingo, guile-devel
[-- Attachment #1: Type: text/plain, Size: 352 bytes --]
So here’s a version that adds a ‘setvbuf’ method to scm_t_port_internal,
and uses that to implement CBIP buffering. CBIPs remain fully buffered
by default (after more thoughts I considered that this conservative
approach was more reasonable and acceptable if documented.)
I’d like to commit to stable-2.0. WDYT?
Thanks,
Ludo’.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: setvbuf method --]
[-- Type: text/x-patch, Size: 8789 bytes --]
From 1c5cc0ad6938443e9dc5271b6974a2c8df3b1909 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= <ludo@gnu.org>
Date: Thu, 16 Jan 2014 23:52:01 +0100
Subject: [PATCH 1/2] Prepare 'setvbuf' to support for non-file ports.
* libguile/ports-internal.h (struct scm_port_internal): Add
setvbuf' field. Change 'pending_eof' to a 1-bit unsigned char.
* libguile/ports.c (scm_new_port_table_entry): Clear 'pti->setvbuf'.
* libguile/fports.c (scm_setvbuf): Accept any open port, and error out
when PORT's setvbuf' field is NULL. Remove explicit 'scm_gc_free' calls.
(scm_i_fdes_to_port): Set PORT's 'setvbuf' field.
* test-suite/tests/ports.test ("setvbuf")["closed port", "string port"]:
New tests.
* doc/ref/posix.texi (Ports and File Descriptors): Suggest that
'setvbuf' works for different port types.
---
doc/ref/posix.texi | 3 +++
libguile/fports.c | 42 +++++++++++++++++++++++++++---------------
libguile/ports-internal.h | 14 ++++++++++++--
libguile/ports.c | 8 +++++++-
test-suite/tests/ports.test | 14 +++++++++++++-
5 files changed, 62 insertions(+), 19 deletions(-)
diff --git a/doc/ref/posix.texi b/doc/ref/posix.texi
index b3a6a04..56f5c78 100644
--- a/doc/ref/posix.texi
+++ b/doc/ref/posix.texi
@@ -470,6 +470,9 @@ line buffered
block buffered, using a newly allocated buffer of @var{size} bytes.
If @var{size} is omitted, a default size will be used.
@end defvar
+
+Only certain types of ports are supported, most importantly
+file ports.
@end deffn
@deffn {Scheme Procedure} fcntl port/fd cmd [value]
diff --git a/libguile/fports.c b/libguile/fports.c
index 70732e5..365d3ff 100644
--- a/libguile/fports.c
+++ b/libguile/fports.c
@@ -1,5 +1,6 @@
/* Copyright (C) 1995, 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003,
- * 2004, 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2013 Free Software Foundation, Inc.
+ * 2004, 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2013,
+ * 2014 Free Software Foundation, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public License
@@ -58,6 +59,7 @@
#include "libguile/hashtab.h"
#include "libguile/fports.h"
+#include "libguile/ports-internal.h"
#if SIZEOF_OFF_T == SIZEOF_INT
#define OFF_T_MAX INT_MAX
@@ -78,10 +80,10 @@ scm_t_bits scm_tc16_fport;
/* default buffer size, used if the O/S won't supply a value. */
static const size_t default_buffer_size = 1024;
-/* create FPORT buffer with specified sizes (or -1 to use default size or
- 0 for no buffer. */
+/* Create FPORT buffers with specified sizes (or -1 to use default size
+ or 0 for no buffer.) */
static void
-scm_fport_buffer_add (SCM port, long read_size, int write_size)
+scm_fport_buffer_add (SCM port, long read_size, long write_size)
#define FUNC_NAME "scm_fport_buffer_add"
{
scm_t_port *pt = SCM_PTAB_ENTRY (port);
@@ -147,7 +149,9 @@ SCM_DEFINE (scm_setvbuf, "setvbuf", 2, 1, 0,
"@item _IOFBF\n"
"block buffered, using a newly allocated buffer of @var{size} bytes.\n"
"If @var{size} is omitted, a default size will be used.\n"
- "@end table")
+ "@end table\n\n"
+ "Only certain types of ports are supported, most importantly\n"
+ "file ports.")
#define FUNC_NAME s_scm_setvbuf
{
int cmode;
@@ -155,10 +159,17 @@ SCM_DEFINE (scm_setvbuf, "setvbuf", 2, 1, 0,
size_t ndrained;
char *drained;
scm_t_port *pt;
+ scm_t_port_internal *pti;
port = SCM_COERCE_OUTPORT (port);
- SCM_VALIDATE_OPFPORT (1,port);
+ SCM_VALIDATE_OPENPORT (1, port);
+ pti = SCM_PORT_GET_INTERNAL (port);
+
+ if (pti->setvbuf == NULL)
+ scm_wrong_type_arg_msg (FUNC_NAME, 1, port,
+ "port that supports 'setvbuf'");
+
cmode = scm_to_int (mode);
if (cmode != _IONBF && cmode != _IOFBF && cmode != _IOLBF)
scm_out_of_range (FUNC_NAME, mode);
@@ -169,9 +180,8 @@ SCM_DEFINE (scm_setvbuf, "setvbuf", 2, 1, 0,
cmode = _IOFBF;
}
else
- {
- SCM_SET_CELL_WORD_0 (port, SCM_CELL_WORD_0 (port) & ~(scm_t_bits)SCM_BUFLINE);
- }
+ SCM_SET_CELL_WORD_0 (port,
+ SCM_CELL_WORD_0 (port) & ~(scm_t_bits) SCM_BUFLINE);
if (SCM_UNBNDP (size))
{
@@ -216,12 +226,8 @@ SCM_DEFINE (scm_setvbuf, "setvbuf", 2, 1, 0,
pt->read_end = pt->saved_read_end;
pt->read_buf_size = pt->saved_read_buf_size;
}
- if (pt->read_buf != &pt->shortbuf)
- scm_gc_free (pt->read_buf, pt->read_buf_size, "port buffer");
- if (pt->write_buf != &pt->shortbuf)
- scm_gc_free (pt->write_buf, pt->write_buf_size, "port buffer");
- scm_fport_buffer_add (port, csize, csize);
+ pti->setvbuf (port, csize, csize);
if (ndrained > 0)
/* Put DRAINED back to PORT. */
@@ -542,6 +548,7 @@ scm_i_fdes_to_port (int fdes, long mode_bits, SCM name)
{
SCM port;
scm_t_port *pt;
+ scm_t_port_internal *pti;
/* Test that fdes is valid. */
#ifdef F_GETFL
@@ -567,7 +574,12 @@ scm_i_fdes_to_port (int fdes, long mode_bits, SCM name)
port = scm_new_port_table_entry (scm_tc16_fport);
SCM_SET_CELL_TYPE(port, scm_tc16_fport | mode_bits);
- pt = SCM_PTAB_ENTRY(port);
+ pt = SCM_PTAB_ENTRY (port);
+
+ /* File ports support 'setvbuf'. */
+ pti = SCM_PORT_GET_INTERNAL (port);
+ pti->setvbuf = scm_fport_buffer_add;
+
{
scm_t_fport *fp
= (scm_t_fport *) scm_gc_malloc_pointerless (sizeof (scm_t_fport),
diff --git a/libguile/ports-internal.h b/libguile/ports-internal.h
index 8a3a00b..48dcaa7 100644
--- a/libguile/ports-internal.h
+++ b/libguile/ports-internal.h
@@ -1,7 +1,7 @@
/*
* ports-internal.h - internal-only declarations for ports.
*
- * Copyright (C) 2013 Free Software Foundation, Inc.
+ * Copyright (C) 2013, 2014 Free Software Foundation, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public License
@@ -50,7 +50,17 @@ struct scm_port_internal
unsigned at_stream_start_for_bom_write : 1;
scm_t_port_encoding_mode encoding_mode;
scm_t_iconv_descriptors *iconv_descriptors;
- int pending_eof;
+ unsigned char pending_eof: 1;
+
+ /* When non-NULL, this is the method called by 'setvbuf' for this port.
+ It must create read and write buffers for PORT with the specified
+ sizes (a size of 0 is for unbuffered ports, which should use the
+ 'shortbuf' field.) Size -1 means to use the port's preferred buffer
+ size. */
+ /* XXX: In 2.2 make this a property of the 'scm_t_ptob_descriptor'. */
+ void (*setvbuf) (SCM port, long read_size, long write_size);
+
+ /* Key-value properties. */
SCM alist;
};
diff --git a/libguile/ports.c b/libguile/ports.c
index 4516160..4f401de 100644
--- a/libguile/ports.c
+++ b/libguile/ports.c
@@ -1,5 +1,5 @@
/* Copyright (C) 1995, 1996, 1997, 1998, 1999, 2000, 2001, 2003, 2004, 2006,
- * 2007, 2008, 2009, 2010, 2011, 2012, 2013 Free Software Foundation, Inc.
+ * 2007, 2008, 2009, 2010, 2011, 2012, 2013, 2014 Free Software Foundation, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public License
@@ -677,6 +677,12 @@ scm_new_port_table_entry (scm_t_bits tag)
pti->pending_eof = 0;
pti->alist = SCM_EOL;
+ /* Until Guile 2.0.9 included, 'setvbuf' would only work on file
+ ports. Now all port types can be supported, but it's not clear
+ that port types out in wild accept having someone else fiddle with
+ their buffer. Thus, conservatively turn it off by default. */
+ pti->setvbuf = NULL;
+
SCM_SET_CELL_TYPE (z, tag);
SCM_SETPTAB_ENTRY (z, entry);
diff --git a/test-suite/tests/ports.test b/test-suite/tests/ports.test
index 9b1c6c0..c2f4480 100644
--- a/test-suite/tests/ports.test
+++ b/test-suite/tests/ports.test
@@ -2,7 +2,7 @@
;;;; Jim Blandy <jimb@red-bean.com> --- May 1999
;;;;
;;;; Copyright (C) 1999, 2001, 2004, 2006, 2007, 2009, 2010,
-;;;; 2011, 2012, 2013 Free Software Foundation, Inc.
+;;;; 2011, 2012, 2013, 2014 Free Software Foundation, Inc.
;;;;
;;;; This library is free software; you can redistribute it and/or
;;;; modify it under the terms of the GNU Lesser General Public
@@ -1499,6 +1499,18 @@
(with-test-prefix "setvbuf"
+ (pass-if-exception "closed port"
+ exception:wrong-type-arg
+ (let ((port (open-input-file "/dev/null")))
+ (close-port port)
+ (setvbuf port _IOFBF)))
+
+ (pass-if-exception "string port"
+ exception:wrong-type-arg
+ (let ((port (open-input-string "Hey!")))
+ (close-port port)
+ (setvbuf port _IOFBF)))
+
(pass-if "line/column number preserved"
;; In Guile 2.0.5, `setvbuf' would erroneously decrease the port's
;; line and/or column number.
--
1.8.4
[-- Attachment #3: unbuffered CBIPs --]
[-- Type: text/x-patch, Size: 13514 bytes --]
From 0fd3ab580b0bf71cf5133744f1b60499c23bbcb6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= <ludo@gnu.org>
Date: Thu, 16 Jan 2014 23:43:31 +0100
Subject: [PATCH 2/2] Custom binary input ports support 'setvbuf'.
* libguile/r6rs-ports.c (CBIP_BUFFER_SIZE): Adjust comment. Set to 8KiB.
(SCM_SET_CBIP_BYTEVECTOR): New macro.
(cbip_setvbuf): New function.
(make_cbip): Set PORT's 'setvbuf' internal field.
(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.
* 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'.
---
doc/ref/api-io.texi | 4 ++
libguile/r6rs-ports.c | 94 +++++++++++++++++++++++++-----
test-suite/tests/r6rs-ports.test | 123 +++++++++++++++++++++++++++++++++++++++
3 files changed, 205 insertions(+), 16 deletions(-)
diff --git a/doc/ref/api-io.texi b/doc/ref/api-io.texi
index f1170eb..02d92a2 100644
--- a/doc/ref/api-io.texi
+++ b/doc/ref/api-io.texi
@@ -1816,6 +1816,10 @@ indicating the position of the next byte is to read.
Finally, if @var{close} is not @code{#f}, it must be a thunk. It is
invoked when the custom binary input port is closed.
+The returned port is fully buffered by default, but its buffering mode
+can be changed using @code{setvbuf} (@pxref{Ports and File Descriptors,
+@code{setvbuf}}).
+
Using a custom binary input port, the @code{open-bytevector-input-port}
procedure could be implemented as follows:
diff --git a/libguile/r6rs-ports.c b/libguile/r6rs-ports.c
index 0b1d162..30456a8 100644
--- a/libguile/r6rs-ports.c
+++ b/libguile/r6rs-ports.c
@@ -37,6 +37,7 @@
#include "libguile/validate.h"
#include "libguile/values.h"
#include "libguile/vectors.h"
+#include "libguile/ports-internal.h"
\f
@@ -280,18 +281,59 @@ cbp_close (SCM port)
static scm_t_bits custom_binary_input_port_type = 0;
-/* Size of the buffer embedded in custom binary input ports. */
-#define CBIP_BUFFER_SIZE 4096
+/* Initial size of the buffer embedded in custom binary input ports. */
+#define CBIP_BUFFER_SIZE 8192
/* Return the bytevector associated with PORT. */
#define SCM_CBIP_BYTEVECTOR(_port) \
SCM_SIMPLE_VECTOR_REF (SCM_PACK (SCM_STREAM (_port)), 4)
+/* Set BV as the bytevector associated with PORT. */
+#define SCM_SET_CBIP_BYTEVECTOR(_port, _bv) \
+ SCM_SIMPLE_VECTOR_SET (SCM_PACK (SCM_STREAM (_port)), 4, (_bv))
+
/* Return the various procedures of PORT. */
#define SCM_CBIP_READ_PROC(_port) \
SCM_SIMPLE_VECTOR_REF (SCM_PACK (SCM_STREAM (_port)), 0)
+/* Set PORT's internal buffer according to READ_SIZE. */
+static void
+cbip_setvbuf (SCM port, long read_size, long write_size)
+{
+ SCM bv;
+ scm_t_port *pt;
+
+ pt = SCM_PTAB_ENTRY (port);
+ bv = SCM_CBIP_BYTEVECTOR (port);
+
+ switch (read_size)
+ {
+ case 0:
+ /* Unbuffered: keep PORT's bytevector as is (it will be used in
+ future 'scm_c_read' calls), but point to the one-byte buffer. */
+ pt->read_buf = &pt->shortbuf;
+ pt->read_buf_size = 1;
+ break;
+
+ case -1:
+ /* Preferred size: keep the current bytevector and use it as the
+ backing store. */
+ pt->read_buf = (unsigned char *) SCM_BYTEVECTOR_CONTENTS (bv);
+ pt->read_buf_size = SCM_BYTEVECTOR_LENGTH (bv);
+ break;
+
+ default:
+ /* Fully buffered: allocate a buffer of READ_SIZE bytes. */
+ bv = scm_c_make_bytevector (read_size);
+ SCM_SET_CBIP_BYTEVECTOR (port, bv);
+ pt->read_buf = (unsigned char *) SCM_BYTEVECTOR_CONTENTS (bv);
+ pt->read_buf_size = read_size;
+ }
+
+ pt->read_pos = pt->read_end = pt->read_buf;
+}
+
static inline SCM
make_cbip (SCM read_proc, SCM get_position_proc,
SCM set_position_proc, SCM close_proc)
@@ -331,7 +373,10 @@ make_cbip (SCM read_proc, SCM get_position_proc,
c_port->read_end = (unsigned char *) c_bv;
c_port->read_buf_size = c_len;
- /* Mark PORT as open, readable and unbuffered (hmm, how elegant...). */
+ /* 'setvbuf' is supported. */
+ SCM_PORT_GET_INTERNAL (port)->setvbuf = cbip_setvbuf;
+
+ /* Mark PORT as open and readable. */
SCM_SET_CELL_TYPE (port, custom_binary_input_port_type | mode_bits);
scm_i_pthread_mutex_unlock (&scm_i_port_table_mutex);
@@ -346,26 +391,39 @@ cbip_fill_input (SCM port)
int result;
scm_t_port *c_port = SCM_PTAB_ENTRY (port);
- again:
if (c_port->read_pos >= c_port->read_end)
{
/* Invoke the user's `read!' procedure. */
+ int buffered;
size_t c_octets, c_requested;
SCM bv, read_proc, octets;
c_requested = c_port->read_buf_size;
+ read_proc = SCM_CBIP_READ_PROC (port);
- /* Use the bytevector associated with PORT as the buffer passed to the
- `read!' procedure, thereby avoiding additional allocations. */
bv = SCM_CBIP_BYTEVECTOR (port);
- read_proc = SCM_CBIP_READ_PROC (port);
+ buffered =
+ (c_port->read_buf == (unsigned char *) SCM_BYTEVECTOR_CONTENTS (bv));
- /* The assumption here is that C_PORT's internal buffer wasn't changed
- behind our back. */
- assert (c_port->read_buf ==
- (unsigned char *) SCM_BYTEVECTOR_CONTENTS (bv));
- assert ((unsigned) c_port->read_buf_size
- == SCM_BYTEVECTOR_LENGTH (bv));
+ if (buffered)
+ /* Make sure the buffer isn't corrupt. BV can be passed directly
+ to READ_PROC. */
+ assert (c_port->read_buf_size == SCM_BYTEVECTOR_LENGTH (bv));
+ else
+ {
+ /* This is an unbuffered port. When called via the
+ 'get-bytevector-*' procedures, and thus via 'scm_c_read', we
+ are passed the caller-provided buffer, so we need to check its
+ size. */
+ if (SCM_BYTEVECTOR_LENGTH (bv) < c_requested)
+ {
+ /* Bad luck: we have to make another allocation. Save that
+ bytevector for later reuse, in the hope that the application
+ has regular access patterns. */
+ bv = scm_c_make_bytevector (c_requested);
+ SCM_SET_CBIP_BYTEVECTOR (port, bv);
+ }
+ }
octets = scm_call_3 (read_proc, bv, SCM_INUM0,
scm_from_size_t (c_requested));
@@ -373,11 +431,15 @@ cbip_fill_input (SCM port)
if (SCM_UNLIKELY (c_octets > c_requested))
scm_out_of_range (FUNC_NAME, octets);
- c_port->read_pos = (unsigned char *) SCM_BYTEVECTOR_CONTENTS (bv);
+ if (!buffered)
+ /* Copy the data back to the internal buffer. */
+ memcpy ((char *) c_port->read_pos, SCM_BYTEVECTOR_CONTENTS (bv),
+ c_octets);
+
c_port->read_end = (unsigned char *) c_port->read_pos + c_octets;
- if (c_octets > 0)
- goto again;
+ if (c_octets != 0 || c_requested == 0)
+ result = (int) *c_port->read_pos;
else
result = EOF;
}
diff --git a/test-suite/tests/r6rs-ports.test b/test-suite/tests/r6rs-ports.test
index 2b62bed..213c8b7 100644
--- a/test-suite/tests/r6rs-ports.test
+++ b/test-suite/tests/r6rs-ports.test
@@ -456,6 +456,129 @@ not `set-port-position!'"
(u8-list->bytevector
(map char->integer (string->list "Port!")))))))
+ (pass-if-equal "custom binary input port unbuffered & 'port-position'"
+ '(0 2 5 11)
+ ;; Check that the value returned by 'port-position' is correct, and
+ ;; that each 'port-position' call leads one call to the
+ ;; 'get-position' method.
+ (let* ((str "Hello Port!")
+ (output (make-bytevector (string-length str)))
+ (source (with-fluids ((%default-port-encoding "UTF-8"))
+ (open-string-input-port str)))
+ (read! (lambda (bv start count)
+ (let ((r (get-bytevector-n! source bv start count)))
+ (if (eof-object? r)
+ 0
+ r))))
+ (pos '())
+ (get-pos (lambda ()
+ (let ((p (port-position source)))
+ (set! pos (cons p pos))
+ p)))
+ (port (make-custom-binary-input-port "the port" read!
+ get-pos #f #f)))
+ (setvbuf port _IONBF)
+ (and (= 0 (port-position port))
+ (begin
+ (get-bytevector-n! port output 0 2)
+ (= 2 (port-position port)))
+ (begin
+ (get-bytevector-n! port output 2 3)
+ (= 5 (port-position port)))
+ (let ((bv (string->utf8 (get-string-all port))))
+ (bytevector-copy! bv 0 output 5 (bytevector-length bv))
+ (= (string-length str) (port-position port)))
+ (bytevector=? output (string->utf8 str))
+ (reverse pos))))
+
+ (pass-if-equal "custom binary input port unbuffered & 'read!' calls"
+ `((2 "He") (3 "llo") (42 " Port!"))
+ (let* ((str "Hello Port!")
+ (source (with-fluids ((%default-port-encoding "UTF-8"))
+ (open-string-input-port str)))
+ (reads '())
+ (read! (lambda (bv start count)
+ (set! reads (cons count reads))
+ (let ((r (get-bytevector-n! source bv start count)))
+ (if (eof-object? r)
+ 0
+ r))))
+ (port (make-custom-binary-input-port "the port" read!
+ #f #f #f)))
+
+ (setvbuf port _IONBF)
+ (let ((ret (list (get-bytevector-n port 2)
+ (get-bytevector-n port 3)
+ (get-bytevector-n port 42))))
+ (zip (reverse reads)
+ (map (lambda (obj)
+ (if (bytevector? obj)
+ (utf8->string obj)
+ obj))
+ ret)))))
+
+ (pass-if-equal "custom binary input port, unbuffered then buffered"
+ `((6 "Lorem ") (12 "ipsum dolor ") (777 "sit amet, consectetur…")
+ (777 ,(eof-object)))
+ (let* ((str "Lorem ipsum dolor sit amet, consectetur…")
+ (source (with-fluids ((%default-port-encoding "UTF-8"))
+ (open-string-input-port str)))
+ (reads '())
+ (read! (lambda (bv start count)
+ (set! reads (cons count reads))
+ (let ((r (get-bytevector-n! source bv start count)))
+ (if (eof-object? r)
+ 0
+ r))))
+ (port (make-custom-binary-input-port "the port" read!
+ #f #f #f)))
+
+ (setvbuf port _IONBF)
+ (let ((ret (list (get-bytevector-n port 6)
+ (get-bytevector-n port 12)
+ (begin
+ (setvbuf port _IOFBF 777)
+ (get-bytevector-n port 42))
+ (get-bytevector-n port 42))))
+ (zip (reverse reads)
+ (map (lambda (obj)
+ (if (bytevector? obj)
+ (utf8->string obj)
+ obj))
+ ret)))))
+
+ (pass-if-equal "custom binary input port, buffered then unbuffered"
+ `((18
+ 42 14 ; scm_c_read tries to fill the 42-byte buffer
+ 42)
+ ("Lorem " "ipsum dolor " "sit amet, consectetur bla…" ,(eof-object)))
+ (let* ((str "Lorem ipsum dolor sit amet, consectetur bla…")
+ (source (with-fluids ((%default-port-encoding "UTF-8"))
+ (open-string-input-port str)))
+ (reads '())
+ (read! (lambda (bv start count)
+ (set! reads (cons count reads))
+ (let ((r (get-bytevector-n! source bv start count)))
+ (if (eof-object? r)
+ 0
+ r))))
+ (port (make-custom-binary-input-port "the port" read!
+ #f #f #f)))
+
+ (setvbuf port _IOFBF 18)
+ (let ((ret (list (get-bytevector-n port 6)
+ (get-bytevector-n port 12)
+ (begin
+ (setvbuf port _IONBF)
+ (get-bytevector-n port 42))
+ (get-bytevector-n port 42))))
+ (list (reverse reads)
+ (map (lambda (obj)
+ (if (bytevector? obj)
+ (utf8->string obj)
+ obj))
+ ret)))))
+
(pass-if "custom binary input port `close-proc' is called"
(let* ((closed? #f)
(read! (lambda (bv start count) 0))
--
1.8.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: Making custom binary input ports unbuffered
2014-01-16 23:00 ` Ludovic Courtès
@ 2014-01-18 21:57 ` Ludovic Courtès
2014-01-21 7:41 ` Mark H Weaver
1 sibling, 0 replies; 11+ messages in thread
From: Ludovic Courtès @ 2014-01-18 21:57 UTC (permalink / raw)
To: guile-devel
ludo@gnu.org (Ludovic Courtès) skribis:
> So here’s a version that adds a ‘setvbuf’ method to scm_t_port_internal,
> and uses that to implement CBIP buffering. CBIPs remain fully buffered
> by default (after more thoughts I considered that this conservative
> approach was more reasonable and acceptable if documented.)
Pushed to ‘stable-2.0’.
Thanks,
Ludo’.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Making custom binary input ports unbuffered
2014-01-16 23:00 ` Ludovic Courtès
2014-01-18 21:57 ` Ludovic Courtès
@ 2014-01-21 7:41 ` Mark H Weaver
2014-01-21 10:50 ` Ludovic Courtès
1 sibling, 1 reply; 11+ messages in thread
From: Mark H Weaver @ 2014-01-21 7:41 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: Andy Wingo, guile-devel
[-- Attachment #1: Type: text/plain, Size: 167 bytes --]
Here are the same changes adapted for master, where we can put the
new 'setvbuf' method where it belongs: in the PTOB.
Comments and suggestions welcome.
Mark
[-- Attachment #2: [PATCH 1/2] Prepare 'setvbuf' to support for non-file ports --]
[-- Type: text/x-patch, Size: 18846 bytes --]
From 00ee913e2da658f30d9d8682edfbb9a63719c370 Mon Sep 17 00:00:00 2001
From: Mark H Weaver <mhw@netris.org>
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.
* 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'.
* doc/ref/posix.texi (Ports and File Descriptors): Suggest that
'setvbuf' works for different port types.
---
doc/ref/api-io.texi | 12 ++++-
doc/ref/posix.texi | 5 ++-
libguile/fports.c | 111 +++-------------------------------------
libguile/fports.h | 5 +-
libguile/ports-internal.h | 6 ++-
libguile/ports.c | 116 ++++++++++++++++++++++++++++++++++++++++++-
libguile/ports.h | 15 +++++-
test-suite/tests/ports.test | 14 +++++-
8 files changed, 172 insertions(+), 112 deletions(-)
diff --git a/doc/ref/api-io.texi b/doc/ref/api-io.texi
index 5ca3506..edf38be 100644
--- a/doc/ref/api-io.texi
+++ b/doc/ref/api-io.texi
@@ -1,7 +1,7 @@
@c -*-texinfo-*-
@c This is part of the GNU Guile Reference Manual.
@c Copyright (C) 1996, 1997, 2000, 2001, 2002, 2003, 2004, 2007, 2009,
-@c 2010, 2011, 2013 Free Software Foundation, Inc.
+@c 2010, 2011, 2013, 2014 Free Software Foundation, Inc.
@c See the file guile.texi for copying conditions.
@node Input and Output
@@ -2403,6 +2403,16 @@ Set using
@deftypefun void scm_set_port_truncate (scm_t_bits tc, void (*truncate) (SCM port, scm_t_off length))
@end deftypefun
+@item 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
+
+@deftypefun void scm_set_port_setvbuf (scm_t_bits tc, void (*setvbuf) (SCM port, long read_size, long write_size))
+@end deftypefun
+
@end table
@node BOM Handling
diff --git a/doc/ref/posix.texi b/doc/ref/posix.texi
index 40c20e7..0ced09b 100644
--- a/doc/ref/posix.texi
+++ b/doc/ref/posix.texi
@@ -1,7 +1,7 @@
@c -*-texinfo-*-
@c This is part of the GNU Guile Reference Manual.
@c Copyright (C) 1996, 1997, 2000, 2001, 2002, 2003, 2004, 2006, 2007,
-@c 2008, 2009, 2010, 2011, 2012, 2013 Free Software Foundation, Inc.
+@c 2008, 2009, 2010, 2011, 2012, 2013, 2014 Free Software Foundation, Inc.
@c See the file guile.texi for copying conditions.
@node POSIX
@@ -470,6 +470,9 @@ line buffered
block buffered, using a newly allocated buffer of @var{size} bytes.
If @var{size} is omitted, a default size will be used.
@end defvar
+
+Only certain types of ports are supported, most importantly
+file ports.
@end deffn
@deffn {Scheme Procedure} fcntl port/fd cmd [value]
diff --git a/libguile/fports.c b/libguile/fports.c
index dc3d45c..9f8f662 100644
--- a/libguile/fports.c
+++ b/libguile/fports.c
@@ -1,5 +1,6 @@
/* Copyright (C) 1995, 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003,
- * 2004, 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2013 Free Software Foundation, Inc.
+ * 2004, 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2013,
+ * 2014 Free Software Foundation, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public License
@@ -78,11 +79,11 @@ scm_t_bits scm_tc16_fport;
/* default buffer size, used if the O/S won't supply a value. */
static const size_t default_buffer_size = 1024;
-/* create FPORT buffer with specified sizes (or -1 to use default size or
- 0 for no buffer. */
+/* Create FPORT buffers with specified sizes (or -1 to use default size
+ or 0 for no buffer.) */
static void
-scm_fport_buffer_add (SCM port, long read_size, int write_size)
-#define FUNC_NAME "scm_fport_buffer_add"
+fport_setvbuf (SCM port, long read_size, long write_size)
+#define FUNC_NAME "fport_setvbuf"
{
scm_t_port *pt = SCM_PTAB_ENTRY (port);
@@ -136,101 +137,6 @@ scm_fport_buffer_add (SCM port, long read_size, int write_size)
}
#undef FUNC_NAME
-SCM_DEFINE (scm_setvbuf, "setvbuf", 2, 1, 0,
- (SCM port, SCM mode, SCM size),
- "Set the buffering mode for @var{port}. @var{mode} can be:\n"
- "@table @code\n"
- "@item _IONBF\n"
- "non-buffered\n"
- "@item _IOLBF\n"
- "line buffered\n"
- "@item _IOFBF\n"
- "block buffered, using a newly allocated buffer of @var{size} bytes.\n"
- "If @var{size} is omitted, a default size will be used.\n"
- "@end table")
-#define FUNC_NAME s_scm_setvbuf
-{
- int cmode;
- long csize;
- size_t ndrained;
- char *drained;
- scm_t_port *pt;
-
- port = SCM_COERCE_OUTPORT (port);
-
- SCM_VALIDATE_OPFPORT (1,port);
- cmode = scm_to_int (mode);
- if (cmode != _IONBF && cmode != _IOFBF && cmode != _IOLBF)
- scm_out_of_range (FUNC_NAME, mode);
-
- if (cmode == _IOLBF)
- {
- SCM_SET_CELL_WORD_0 (port, SCM_CELL_WORD_0 (port) | SCM_BUFLINE);
- cmode = _IOFBF;
- }
- else
- {
- SCM_SET_CELL_WORD_0 (port, SCM_CELL_WORD_0 (port) & ~(scm_t_bits)SCM_BUFLINE);
- }
-
- if (SCM_UNBNDP (size))
- {
- if (cmode == _IOFBF)
- csize = -1;
- else
- csize = 0;
- }
- else
- {
- csize = scm_to_int (size);
- if (csize < 0 || (cmode == _IONBF && csize > 0))
- scm_out_of_range (FUNC_NAME, size);
- }
-
- pt = SCM_PTAB_ENTRY (port);
-
- if (SCM_INPUT_PORT_P (port))
- {
- /* Drain pending input from PORT. Don't use `scm_drain_input' since
- it returns a string, whereas we want binary input here. */
- ndrained = pt->read_end - pt->read_pos;
- if (pt->read_buf == pt->putback_buf)
- ndrained += pt->saved_read_end - pt->saved_read_pos;
-
- if (ndrained > 0)
- {
- drained = scm_gc_malloc_pointerless (ndrained, "file port");
- scm_take_from_input_buffers (port, drained, ndrained);
- }
- }
- else
- ndrained = 0;
-
- if (SCM_OUTPUT_PORT_P (port))
- scm_flush_unlocked (port);
-
- if (pt->read_buf == pt->putback_buf)
- {
- pt->read_buf = pt->saved_read_buf;
- pt->read_pos = pt->saved_read_pos;
- pt->read_end = pt->saved_read_end;
- pt->read_buf_size = pt->saved_read_buf_size;
- }
- if (pt->read_buf != &pt->shortbuf)
- scm_gc_free (pt->read_buf, pt->read_buf_size, "port buffer");
- if (pt->write_buf != &pt->shortbuf)
- scm_gc_free (pt->write_buf, pt->write_buf_size, "port buffer");
-
- scm_fport_buffer_add (port, csize, csize);
-
- if (ndrained > 0)
- /* Put DRAINED back to PORT. */
- scm_unget_bytes ((unsigned char *) drained, ndrained, port);
-
- return SCM_UNSPECIFIED;
-}
-#undef FUNC_NAME
-
/* Move ports with the specified file descriptor to new descriptors,
* resetting the revealed count to 0.
*/
@@ -574,9 +480,9 @@ scm_i_fdes_to_port (int fdes, long mode_bits, SCM name)
SCM_PTAB_ENTRY (port)->rw_random = SCM_FDES_RANDOM_P (fdes);
if (mode_bits & SCM_BUF0)
- scm_fport_buffer_add (port, 0, 0);
+ fport_setvbuf (port, 0, 0);
else
- scm_fport_buffer_add (port, -1, -1);
+ fport_setvbuf (port, -1, -1);
SCM_SET_FILENAME (port, name);
@@ -974,6 +880,7 @@ scm_make_fptob ()
scm_set_port_seek (tc, fport_seek);
scm_set_port_truncate (tc, fport_truncate);
scm_set_port_input_waiting (tc, fport_input_waiting);
+ scm_set_port_setvbuf (tc, fport_setvbuf);
return tc;
}
diff --git a/libguile/fports.h b/libguile/fports.h
index 092b43e..6eb2dd9 100644
--- a/libguile/fports.h
+++ b/libguile/fports.h
@@ -3,7 +3,8 @@
#ifndef SCM_FPORTS_H
#define SCM_FPORTS_H
-/* Copyright (C) 1995,1996,1997,1998,1999,2000,2001, 2006, 2008, 2009, 2011, 2012 Free Software Foundation, Inc.
+/* Copyright (C) 1995,1996,1997,1998,1999,2000,2001, 2006, 2008, 2009,
+ * 2011, 2012, 2014 Free Software Foundation, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public License
@@ -51,8 +52,6 @@ SCM_API scm_t_bits scm_tc16_fport;
#define SCM_FDES_RANDOM_P(fdes) ((lseek (fdes, 0, SEEK_CUR) == -1) ? 0 : 1)
\f
-SCM_API SCM scm_setbuf0 (SCM port);
-SCM_API SCM scm_setvbuf (SCM port, SCM mode, SCM size);
SCM_API void scm_evict_ports (int fd);
SCM_API SCM scm_open_file_with_encoding (SCM filename, SCM modes,
SCM guess_encoding, SCM encoding);
diff --git a/libguile/ports-internal.h b/libguile/ports-internal.h
index bff89cb..963dc21 100644
--- a/libguile/ports-internal.h
+++ b/libguile/ports-internal.h
@@ -1,7 +1,7 @@
/*
* ports-internal.h - internal-only declarations for ports.
*
- * Copyright (C) 2013 Free Software Foundation, Inc.
+ * Copyright (C) 2013, 2014 Free Software Foundation, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public License
@@ -48,9 +48,11 @@ struct scm_port_internal
{
unsigned at_stream_start_for_bom_read : 1;
unsigned at_stream_start_for_bom_write : 1;
+ unsigned pending_eof : 1;
scm_t_port_encoding_mode encoding_mode;
scm_t_iconv_descriptors *iconv_descriptors;
- int pending_eof;
+
+ /* Key-value properties. */
SCM alist;
};
diff --git a/libguile/ports.c b/libguile/ports.c
index d8d27cc..dfab72b 100644
--- a/libguile/ports.c
+++ b/libguile/ports.c
@@ -1,5 +1,6 @@
/* Copyright (C) 1995, 1996, 1997, 1998, 1999, 2000, 2001, 2003, 2004,
- * 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2013 Free Software Foundation, Inc.
+ * 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2013,
+ * 2014 Free Software Foundation, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public License
@@ -259,6 +260,12 @@ scm_make_port_type (char *name,
desc->end_input = end_input_default;
desc->fill_input = fill_input;
+ /* Until Guile 2.0.10, 'setvbuf' would only work on file ports. Now
+ all port types can be supported, but it's not clear that port types
+ out in wild accept having someone else fiddle with their buffer.
+ Thus, conservatively turn it off by default. */
+ desc->setvbuf = NULL;
+
ptobnum = scm_c_port_type_add_x (desc);
/* Make a class object if GOOPS is present. */
@@ -331,6 +338,14 @@ scm_set_port_input_waiting (scm_t_bits tc, int (*input_waiting) (SCM))
scm_c_port_type_ref (SCM_TC2PTOBNUM (tc))->input_waiting = input_waiting;
}
+void
+scm_set_port_setvbuf (scm_t_bits tc, void (*setvbuf) (SCM port,
+ long read_size,
+ long write_size))
+{
+ scm_c_port_type_ref (SCM_TC2PTOBNUM (tc))->setvbuf = setvbuf;
+}
+
static void
scm_i_set_pending_eof (SCM port)
{
@@ -2329,6 +2344,105 @@ SCM_DEFINE (scm_unread_string, "unread-string", 2, 0, 0,
/* Manipulating the buffers. */
+SCM_DEFINE (scm_setvbuf, "setvbuf", 2, 1, 0,
+ (SCM port, SCM mode, SCM size),
+ "Set the buffering mode for @var{port}. @var{mode} can be:\n"
+ "@table @code\n"
+ "@item _IONBF\n"
+ "non-buffered\n"
+ "@item _IOLBF\n"
+ "line buffered\n"
+ "@item _IOFBF\n"
+ "block buffered, using a newly allocated buffer of @var{size} bytes.\n"
+ "If @var{size} is omitted, a default size will be used.\n"
+ "@end table\n\n"
+ "Only certain types of ports are supported, most importantly\n"
+ "file ports.")
+#define FUNC_NAME s_scm_setvbuf
+{
+ int cmode;
+ long csize;
+ size_t ndrained;
+ char *drained;
+ scm_t_port *pt;
+ scm_t_ptob_descriptor *ptob;
+
+ port = SCM_COERCE_OUTPORT (port);
+
+ SCM_VALIDATE_OPENPORT (1, port);
+
+ ptob = SCM_PORT_DESCRIPTOR (port);
+ if (ptob->setvbuf == NULL)
+ scm_wrong_type_arg_msg (FUNC_NAME, 1, port,
+ "port that supports 'setvbuf'");
+
+ cmode = scm_to_int (mode);
+ if (cmode != _IONBF && cmode != _IOFBF && cmode != _IOLBF)
+ scm_out_of_range (FUNC_NAME, mode);
+
+ if (cmode == _IOLBF)
+ {
+ SCM_SET_CELL_WORD_0 (port, SCM_CELL_WORD_0 (port) | SCM_BUFLINE);
+ cmode = _IOFBF;
+ }
+ else
+ SCM_SET_CELL_WORD_0 (port,
+ SCM_CELL_WORD_0 (port) & ~(scm_t_bits) SCM_BUFLINE);
+
+ if (SCM_UNBNDP (size))
+ {
+ if (cmode == _IOFBF)
+ csize = -1;
+ else
+ csize = 0;
+ }
+ else
+ {
+ csize = scm_to_int (size);
+ if (csize < 0 || (cmode == _IONBF && csize > 0))
+ scm_out_of_range (FUNC_NAME, size);
+ }
+
+ pt = SCM_PTAB_ENTRY (port);
+
+ if (SCM_INPUT_PORT_P (port))
+ {
+ /* Drain pending input from PORT. Don't use `scm_drain_input' since
+ it returns a string, whereas we want binary input here. */
+ ndrained = pt->read_end - pt->read_pos;
+ if (pt->read_buf == pt->putback_buf)
+ ndrained += pt->saved_read_end - pt->saved_read_pos;
+
+ if (ndrained > 0)
+ {
+ drained = scm_gc_malloc_pointerless (ndrained, "file port");
+ scm_take_from_input_buffers (port, drained, ndrained);
+ }
+ }
+ else
+ ndrained = 0;
+
+ if (SCM_OUTPUT_PORT_P (port))
+ scm_flush_unlocked (port);
+
+ if (pt->read_buf == pt->putback_buf)
+ {
+ pt->read_buf = pt->saved_read_buf;
+ pt->read_pos = pt->saved_read_pos;
+ pt->read_end = pt->saved_read_end;
+ pt->read_buf_size = pt->saved_read_buf_size;
+ }
+
+ ptob->setvbuf (port, csize, csize);
+
+ if (ndrained > 0)
+ /* Put DRAINED back to PORT. */
+ scm_unget_bytes ((unsigned char *) drained, ndrained, port);
+
+ return SCM_UNSPECIFIED;
+}
+#undef FUNC_NAME
+
/* This routine does not take any locks, as it is usually called as part
of a port implementation. */
void
diff --git a/libguile/ports.h b/libguile/ports.h
index a7fde39..7ecdd71 100644
--- a/libguile/ports.h
+++ b/libguile/ports.h
@@ -4,7 +4,8 @@
#define SCM_PORTS_H
/* Copyright (C) 1995, 1996, 1997, 1998, 1999, 2000, 2001, 2003, 2004,
- * 2006, 2008, 2009, 2010, 2011, 2012, 2013 Free Software Foundation, Inc.
+ * 2006, 2008, 2009, 2010, 2011, 2012, 2013,
+ * 2014 Free Software Foundation, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public License
@@ -200,6 +201,13 @@ typedef struct scm_t_ptob_descriptor
scm_t_off (*seek) (SCM port, scm_t_off OFFSET, int WHENCE);
void (*truncate) (SCM port, scm_t_off length);
+ /* When non-NULL, this is the method called by 'setvbuf' for this port.
+ It must create read and write buffers for PORT with the specified
+ sizes (a size of 0 is for unbuffered ports, which should use the
+ 'shortbuf' field.) Size -1 means to use the port's preferred buffer
+ size. */
+ void (*setvbuf) (SCM port, long read_size, long write_size);
+
unsigned flags;
} scm_t_ptob_descriptor;
@@ -238,6 +246,10 @@ SCM_API void scm_set_port_truncate (scm_t_bits tc,
void (*truncate) (SCM port,
scm_t_off length));
SCM_API void scm_set_port_input_waiting (scm_t_bits tc, int (*input_waiting) (SCM));
+SCM_API void scm_set_port_setvbuf (scm_t_bits tc,
+ void (*setvbuf) (SCM port,
+ long read_size,
+ long write_size));
/* The input, output, error, and load ports. */
SCM_API SCM scm_current_input_port (void);
@@ -328,6 +340,7 @@ SCM_API SCM scm_unread_char (SCM cobj, SCM port);
SCM_API SCM scm_unread_string (SCM str, SCM port);
/* Manipulating the buffers. */
+SCM_API SCM scm_setvbuf (SCM port, SCM mode, SCM size);
SCM_API void scm_port_non_buffer (scm_t_port *pt);
SCM_API int scm_fill_input (SCM port);
SCM_API int scm_fill_input_unlocked (SCM port);
diff --git a/test-suite/tests/ports.test b/test-suite/tests/ports.test
index 3d0bba5..bad4118 100644
--- a/test-suite/tests/ports.test
+++ b/test-suite/tests/ports.test
@@ -2,7 +2,7 @@
;;;; Jim Blandy <jimb@red-bean.com> --- May 1999
;;;;
;;;; Copyright (C) 1999, 2001, 2004, 2006, 2007, 2009, 2010,
-;;;; 2011, 2012, 2013 Free Software Foundation, Inc.
+;;;; 2011, 2012, 2013, 2014 Free Software Foundation, Inc.
;;;;
;;;; This library is free software; you can redistribute it and/or
;;;; modify it under the terms of the GNU Lesser General Public
@@ -1468,6 +1468,18 @@
(with-test-prefix "setvbuf"
+ (pass-if-exception "closed port"
+ exception:wrong-type-arg
+ (let ((port (open-input-file "/dev/null")))
+ (close-port port)
+ (setvbuf port _IOFBF)))
+
+ (pass-if-exception "string port"
+ exception:wrong-type-arg
+ (let ((port (open-input-string "Hey!")))
+ (close-port port)
+ (setvbuf port _IOFBF)))
+
(pass-if "line/column number preserved"
;; In Guile 2.0.5, `setvbuf' would erroneously decrease the port's
;; line and/or column number.
--
1.7.5.4
[-- Attachment #3: [PATCH 2/2] Custom binary input ports support 'setvbuf' --]
[-- Type: text/x-patch, Size: 13193 bytes --]
From 6a3feb79b289410a62d2e0c8a70e0ea59d0cf8cd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= <ludo@gnu.org>
Date: Tue, 21 Jan 2014 02:28:35 -0500
Subject: [PATCH 2/2] Custom binary input ports support 'setvbuf'.
Modified-by: Mark H Weaver <mhw@netris.org>
* 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'.
---
doc/ref/api-io.texi | 4 +
libguile/r6rs-ports.c | 89 +++++++++++++++++++++++-----
test-suite/tests/r6rs-ports.test | 123 ++++++++++++++++++++++++++++++++++++++
3 files changed, 201 insertions(+), 15 deletions(-)
diff --git a/doc/ref/api-io.texi b/doc/ref/api-io.texi
index edf38be..49e2a05 100644
--- a/doc/ref/api-io.texi
+++ b/doc/ref/api-io.texi
@@ -1792,6 +1792,10 @@ indicating the position of the next byte is to read.
Finally, if @var{close} is not @code{#f}, it must be a thunk. It is
invoked when the custom binary input port is closed.
+The returned port is fully buffered by default, but its buffering mode
+can be changed using @code{setvbuf} (@pxref{Ports and File Descriptors,
+@code{setvbuf}}).
+
Using a custom binary input port, the @code{open-bytevector-input-port}
procedure could be implemented as follows:
diff --git a/libguile/r6rs-ports.c b/libguile/r6rs-ports.c
index a8fc3f1..5b8da1d 100644
--- a/libguile/r6rs-ports.c
+++ b/libguile/r6rs-ports.c
@@ -272,18 +272,59 @@ cbp_close (SCM port)
static scm_t_bits custom_binary_input_port_type = 0;
-/* Size of the buffer embedded in custom binary input ports. */
-#define CBIP_BUFFER_SIZE 4096
+/* Initial size of the buffer embedded in custom binary input ports. */
+#define CBIP_BUFFER_SIZE 8192
/* Return the bytevector associated with PORT. */
#define SCM_CBIP_BYTEVECTOR(_port) \
SCM_SIMPLE_VECTOR_REF (SCM_PACK (SCM_STREAM (_port)), 4)
+/* Set BV as the bytevector associated with PORT. */
+#define SCM_SET_CBIP_BYTEVECTOR(_port, _bv) \
+ SCM_SIMPLE_VECTOR_SET (SCM_PACK (SCM_STREAM (_port)), 4, (_bv))
+
/* Return the various procedures of PORT. */
#define SCM_CBIP_READ_PROC(_port) \
SCM_SIMPLE_VECTOR_REF (SCM_PACK (SCM_STREAM (_port)), 0)
+/* Set PORT's internal buffer according to READ_SIZE. */
+static void
+cbip_setvbuf (SCM port, long read_size, long write_size)
+{
+ SCM bv;
+ scm_t_port *pt;
+
+ pt = SCM_PTAB_ENTRY (port);
+ bv = SCM_CBIP_BYTEVECTOR (port);
+
+ switch (read_size)
+ {
+ case 0:
+ /* Unbuffered: keep PORT's bytevector as is (it will be used in
+ future 'scm_c_read' calls), but point to the one-byte buffer. */
+ pt->read_buf = &pt->shortbuf;
+ pt->read_buf_size = 1;
+ break;
+
+ case -1:
+ /* Preferred size: keep the current bytevector and use it as the
+ backing store. */
+ pt->read_buf = (unsigned char *) SCM_BYTEVECTOR_CONTENTS (bv);
+ pt->read_buf_size = SCM_BYTEVECTOR_LENGTH (bv);
+ break;
+
+ default:
+ /* Fully buffered: allocate a buffer of READ_SIZE bytes. */
+ bv = scm_c_make_bytevector (read_size);
+ SCM_SET_CBIP_BYTEVECTOR (port, bv);
+ pt->read_buf = (unsigned char *) SCM_BYTEVECTOR_CONTENTS (bv);
+ pt->read_buf_size = read_size;
+ }
+
+ pt->read_pos = pt->read_end = pt->read_buf;
+}
+
static inline SCM
make_cbip (SCM read_proc, SCM get_position_proc,
SCM set_position_proc, SCM close_proc)
@@ -330,26 +371,39 @@ cbip_fill_input (SCM port)
int result;
scm_t_port *c_port = SCM_PTAB_ENTRY (port);
- again:
if (c_port->read_pos >= c_port->read_end)
{
/* Invoke the user's `read!' procedure. */
+ int buffered;
size_t c_octets, c_requested;
SCM bv, read_proc, octets;
c_requested = c_port->read_buf_size;
+ read_proc = SCM_CBIP_READ_PROC (port);
- /* Use the bytevector associated with PORT as the buffer passed to the
- `read!' procedure, thereby avoiding additional allocations. */
bv = SCM_CBIP_BYTEVECTOR (port);
- read_proc = SCM_CBIP_READ_PROC (port);
+ buffered =
+ (c_port->read_buf == (unsigned char *) SCM_BYTEVECTOR_CONTENTS (bv));
- /* The assumption here is that C_PORT's internal buffer wasn't changed
- behind our back. */
- assert (c_port->read_buf ==
- (unsigned char *) SCM_BYTEVECTOR_CONTENTS (bv));
- assert ((unsigned) c_port->read_buf_size
- == SCM_BYTEVECTOR_LENGTH (bv));
+ if (buffered)
+ /* Make sure the buffer isn't corrupt. BV can be passed directly
+ to READ_PROC. */
+ assert (c_port->read_buf_size == SCM_BYTEVECTOR_LENGTH (bv));
+ else
+ {
+ /* This is an unbuffered port. When called via the
+ 'get-bytevector-*' procedures, and thus via 'scm_c_read', we
+ are passed the caller-provided buffer, so we need to check its
+ size. */
+ if (SCM_BYTEVECTOR_LENGTH (bv) < c_requested)
+ {
+ /* Bad luck: we have to make another allocation. Save that
+ bytevector for later reuse, in the hope that the application
+ has regular access patterns. */
+ bv = scm_c_make_bytevector (c_requested);
+ SCM_SET_CBIP_BYTEVECTOR (port, bv);
+ }
+ }
octets = scm_call_3 (read_proc, bv, SCM_INUM0,
scm_from_size_t (c_requested));
@@ -357,11 +411,15 @@ cbip_fill_input (SCM port)
if (SCM_UNLIKELY (c_octets > c_requested))
scm_out_of_range (FUNC_NAME, octets);
- c_port->read_pos = (unsigned char *) SCM_BYTEVECTOR_CONTENTS (bv);
+ if (!buffered)
+ /* Copy the data back to the internal buffer. */
+ memcpy ((char *) c_port->read_pos, SCM_BYTEVECTOR_CONTENTS (bv),
+ c_octets);
+
c_port->read_end = (unsigned char *) c_port->read_pos + c_octets;
- if (c_octets > 0)
- goto again;
+ if (c_octets != 0 || c_requested == 0)
+ result = (int) *c_port->read_pos;
else
result = EOF;
}
@@ -410,6 +468,7 @@ initialize_custom_binary_input_ports (void)
scm_set_port_seek (custom_binary_input_port_type, cbp_seek);
scm_set_port_close (custom_binary_input_port_type, cbp_close);
+ scm_set_port_setvbuf (custom_binary_input_port_type, cbip_setvbuf);
}
diff --git a/test-suite/tests/r6rs-ports.test b/test-suite/tests/r6rs-ports.test
index 4bd8a70..339679f 100644
--- a/test-suite/tests/r6rs-ports.test
+++ b/test-suite/tests/r6rs-ports.test
@@ -456,6 +456,129 @@ not `set-port-position!'"
(u8-list->bytevector
(map char->integer (string->list "Port!")))))))
+ (pass-if-equal "custom binary input port unbuffered & 'port-position'"
+ '(0 2 5 11)
+ ;; Check that the value returned by 'port-position' is correct, and
+ ;; that each 'port-position' call leads one call to the
+ ;; 'get-position' method.
+ (let* ((str "Hello Port!")
+ (output (make-bytevector (string-length str)))
+ (source (with-fluids ((%default-port-encoding "UTF-8"))
+ (open-string-input-port str)))
+ (read! (lambda (bv start count)
+ (let ((r (get-bytevector-n! source bv start count)))
+ (if (eof-object? r)
+ 0
+ r))))
+ (pos '())
+ (get-pos (lambda ()
+ (let ((p (port-position source)))
+ (set! pos (cons p pos))
+ p)))
+ (port (make-custom-binary-input-port "the port" read!
+ get-pos #f #f)))
+ (setvbuf port _IONBF)
+ (and (= 0 (port-position port))
+ (begin
+ (get-bytevector-n! port output 0 2)
+ (= 2 (port-position port)))
+ (begin
+ (get-bytevector-n! port output 2 3)
+ (= 5 (port-position port)))
+ (let ((bv (string->utf8 (get-string-all port))))
+ (bytevector-copy! bv 0 output 5 (bytevector-length bv))
+ (= (string-length str) (port-position port)))
+ (bytevector=? output (string->utf8 str))
+ (reverse pos))))
+
+ (pass-if-equal "custom binary input port unbuffered & 'read!' calls"
+ `((2 "He") (3 "llo") (42 " Port!"))
+ (let* ((str "Hello Port!")
+ (source (with-fluids ((%default-port-encoding "UTF-8"))
+ (open-string-input-port str)))
+ (reads '())
+ (read! (lambda (bv start count)
+ (set! reads (cons count reads))
+ (let ((r (get-bytevector-n! source bv start count)))
+ (if (eof-object? r)
+ 0
+ r))))
+ (port (make-custom-binary-input-port "the port" read!
+ #f #f #f)))
+
+ (setvbuf port _IONBF)
+ (let ((ret (list (get-bytevector-n port 2)
+ (get-bytevector-n port 3)
+ (get-bytevector-n port 42))))
+ (zip (reverse reads)
+ (map (lambda (obj)
+ (if (bytevector? obj)
+ (utf8->string obj)
+ obj))
+ ret)))))
+
+ (pass-if-equal "custom binary input port, unbuffered then buffered"
+ `((6 "Lorem ") (12 "ipsum dolor ") (777 "sit amet, consectetur…")
+ (777 ,(eof-object)))
+ (let* ((str "Lorem ipsum dolor sit amet, consectetur…")
+ (source (with-fluids ((%default-port-encoding "UTF-8"))
+ (open-string-input-port str)))
+ (reads '())
+ (read! (lambda (bv start count)
+ (set! reads (cons count reads))
+ (let ((r (get-bytevector-n! source bv start count)))
+ (if (eof-object? r)
+ 0
+ r))))
+ (port (make-custom-binary-input-port "the port" read!
+ #f #f #f)))
+
+ (setvbuf port _IONBF)
+ (let ((ret (list (get-bytevector-n port 6)
+ (get-bytevector-n port 12)
+ (begin
+ (setvbuf port _IOFBF 777)
+ (get-bytevector-n port 42))
+ (get-bytevector-n port 42))))
+ (zip (reverse reads)
+ (map (lambda (obj)
+ (if (bytevector? obj)
+ (utf8->string obj)
+ obj))
+ ret)))))
+
+ (pass-if-equal "custom binary input port, buffered then unbuffered"
+ `((18
+ 42 14 ; scm_c_read tries to fill the 42-byte buffer
+ 42)
+ ("Lorem " "ipsum dolor " "sit amet, consectetur bla…" ,(eof-object)))
+ (let* ((str "Lorem ipsum dolor sit amet, consectetur bla…")
+ (source (with-fluids ((%default-port-encoding "UTF-8"))
+ (open-string-input-port str)))
+ (reads '())
+ (read! (lambda (bv start count)
+ (set! reads (cons count reads))
+ (let ((r (get-bytevector-n! source bv start count)))
+ (if (eof-object? r)
+ 0
+ r))))
+ (port (make-custom-binary-input-port "the port" read!
+ #f #f #f)))
+
+ (setvbuf port _IOFBF 18)
+ (let ((ret (list (get-bytevector-n port 6)
+ (get-bytevector-n port 12)
+ (begin
+ (setvbuf port _IONBF)
+ (get-bytevector-n port 42))
+ (get-bytevector-n port 42))))
+ (list (reverse reads)
+ (map (lambda (obj)
+ (if (bytevector? obj)
+ (utf8->string obj)
+ obj))
+ ret)))))
+
(pass-if "custom binary input port `close-proc' is called"
(let* ((closed? #f)
(read! (lambda (bv start count) 0))
--
1.7.5.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: Making custom binary input ports unbuffered
2014-01-21 7:41 ` Mark H Weaver
@ 2014-01-21 10:50 ` Ludovic Courtès
2014-01-21 14:46 ` Mark H Weaver
0 siblings, 1 reply; 11+ messages in thread
From: Ludovic Courtès @ 2014-01-21 10:50 UTC (permalink / raw)
To: Mark H Weaver; +Cc: Andy Wingo, guile-devel
Mark H Weaver <mhw@netris.org> 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 <mhw@netris.org>
> 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?
> +@item 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
> +
> +@deftypefun void scm_set_port_setvbuf (scm_t_bits tc, void (*setvbuf) (SCM port, long read_size, long write_size))
> +@end 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?= <ludo@gnu.org>
> Date: Tue, 21 Jan 2014 02:28:35 -0500
> Subject: [PATCH 2/2] Custom binary input ports support 'setvbuf'.
>
> Modified-by: Mark H Weaver <mhw@netris.org>
>
> * 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’.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Making custom binary input ports unbuffered
2014-01-21 10:50 ` Ludovic Courtès
@ 2014-01-21 14:46 ` Mark H Weaver
2014-01-21 17:37 ` Ludovic Courtès
0 siblings, 1 reply; 11+ messages in thread
From: Mark H Weaver @ 2014-01-21 14:46 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: Andy Wingo, guile-devel
Hi Ludovic,
Thanks for the quick review!
ludo@gnu.org (Ludovic Courtès) writes:
> Mark H Weaver <mhw@netris.org> 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?
That's initially what I started to do, but merging this patch to master
was more work than I expected. There are massive changes to the ports
code in master. It involved asking questions like "Are there any new
ways to create file ports in master?", and involved several merge
failures for changes that will be immediately and completely undone
anyway. I found myself asking "Why am I doing this?"
If you want to do it like you suggest, would you be willing to handle
the merge of your setvbuf patches to master? I've already merged
everything else.
Anyway, please see below for additional responses to your comments.
>> From 00ee913e2da658f30d9d8682edfbb9a63719c370 Mon Sep 17 00:00:00 2001
>> From: Mark H Weaver <mhw@netris.org>
>> 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.)
Okay. Other changes that could be done in 2.0 include renaming
'scm_fport_buffer_add' to 'fport_setvbuf' and removing the prototype for
'scm_setbuf0'.
>
>> * 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?
Hmm. I'm not 100% sure of it either. Unfortunately, we cannot avoid
exposing it as soon as it goes into the PTOB, since the PTOB structure
is public.
Can we discuss the API now and reach a decision?
> Perhaps it’s also better for a separate patch?
What as a separate patch? Do you mean that we should augment the PTOB
in one patch and not document it until a later patch?
Thanks!
Mark
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Making custom binary input ports unbuffered
2014-01-21 14:46 ` Mark H Weaver
@ 2014-01-21 17:37 ` Ludovic Courtès
0 siblings, 0 replies; 11+ messages in thread
From: Ludovic Courtès @ 2014-01-21 17:37 UTC (permalink / raw)
To: Mark H Weaver; +Cc: Andy Wingo, guile-devel
Mark H Weaver <mhw@netris.org> skribis:
> ludo@gnu.org (Ludovic Courtès) writes:
>
>> Mark H Weaver <mhw@netris.org> 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?
>
> That's initially what I started to do, but merging this patch to master
> was more work than I expected. There are massive changes to the ports
> code in master. It involved asking questions like "Are there any new
> ways to create file ports in master?", and involved several merge
> failures for changes that will be immediately and completely undone
> anyway. I found myself asking "Why am I doing this?"
OK.
> If you want to do it like you suggest, would you be willing to handle
> the merge of your setvbuf patches to master? I've already merged
> everything else.
I trust your judgment on this. :-)
>>> From 00ee913e2da658f30d9d8682edfbb9a63719c370 Mon Sep 17 00:00:00 2001
>>> From: Mark H Weaver <mhw@netris.org>
>>> 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.)
>
> Okay. Other changes that could be done in 2.0 include renaming
> 'scm_fport_buffer_add' to 'fport_setvbuf' and removing the prototype for
> 'scm_setbuf0'.
Right.
>>> * 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?
>
> Hmm. I'm not 100% sure of it either. Unfortunately, we cannot avoid
> exposing it as soon as it goes into the PTOB, since the PTOB structure
> is public.
Argh, right. I would be ideal if scm_t_ptob_descriptor were private
too, but I guess we cannot really do this now. Maybe we could deprecate
it for external use in master (with an SCM_DEPRECATED attribute)?
> Can we discuss the API now and reach a decision?
Yes. (I don’t have much to say though, because it looked OKish, but I
just wasn’t sure if it was the Right Way.)
>> Perhaps it’s also better for a separate patch?
>
> What as a separate patch? Do you mean that we should augment the PTOB
> in one patch and not document it until a later patch?
I was referring to the addition of ‘scm_set_port_setvbuf’, but I
understand your point now.
Thanks,
Ludo’.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2014-01-21 17:37 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-14 23:00 Making custom binary input ports unbuffered Ludovic Courtès
2014-01-15 5:43 ` Mark H Weaver
2014-01-15 11:48 ` Ludovic Courtès
2014-01-15 22:51 ` Ludovic Courtès
2014-01-16 0:15 ` Mark H Weaver
2014-01-16 23:00 ` Ludovic Courtès
2014-01-18 21:57 ` Ludovic Courtès
2014-01-21 7:41 ` Mark H Weaver
2014-01-21 10:50 ` Ludovic Courtès
2014-01-21 14:46 ` Mark H Weaver
2014-01-21 17:37 ` Ludovic Courtès
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).