From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Mark H Weaver Newsgroups: gmane.lisp.guile.devel Subject: Re: [PATCH] Fix `get-string-n!' &i/o-decoding exception behavior Date: Sun, 11 Nov 2012 02:26:59 -0500 Message-ID: <874nkwa9bw.fsf@tines.lan> References: <1352332308-5191-1-git-send-email-a.rottmann@gmx.at> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: ger.gmane.org 1352618855 23757 80.91.229.3 (11 Nov 2012 07:27:35 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sun, 11 Nov 2012 07:27:35 +0000 (UTC) Cc: guile-devel@gnu.org To: Andreas Rottmann Original-X-From: guile-devel-bounces+guile-devel=m.gmane.org@gnu.org Sun Nov 11 08:27:45 2012 Return-path: Envelope-to: guile-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1TXRxI-0007WH-Fq for guile-devel@m.gmane.org; Sun, 11 Nov 2012 08:27:44 +0100 Original-Received: from localhost ([::1]:54000 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TXRx9-0002v7-0g for guile-devel@m.gmane.org; Sun, 11 Nov 2012 02:27:35 -0500 Original-Received: from eggs.gnu.org ([208.118.235.92]:52390) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TXRx3-0002v2-OB for guile-devel@gnu.org; Sun, 11 Nov 2012 02:27:32 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TXRx0-0006A5-Lc for guile-devel@gnu.org; Sun, 11 Nov 2012 02:27:29 -0500 Original-Received: from world.peace.net ([96.39.62.75]:33984) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TXRx0-00069W-GE for guile-devel@gnu.org; Sun, 11 Nov 2012 02:27:26 -0500 Original-Received: from 209-6-91-212.c3-0.smr-ubr1.sbo-smr.ma.cable.rcn.com ([209.6.91.212] helo=tines.lan) by world.peace.net with esmtpsa (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:16) (Exim 4.72) (envelope-from ) id 1TXRwh-0000Lh-8s; Sun, 11 Nov 2012 02:27:07 -0500 In-Reply-To: <1352332308-5191-1-git-send-email-a.rottmann@gmx.at> (Andreas Rottmann's message of "Thu, 8 Nov 2012 00:51:48 +0100") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.2 (gnu/linux) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x X-Received-From: 96.39.62.75 X-BeenThere: guile-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Developers list for Guile, the GNU extensibility library" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guile-devel-bounces+guile-devel=m.gmane.org@gnu.org Original-Sender: guile-devel-bounces+guile-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.lisp.guile.devel:15150 Archived-At: Hi Andreas, Thanks for the patch. See below for my comments. Andreas Rottmann writes: > Previously, `get-string-n!' from `(rnrs io ports)' would not throw the > exception required by R6RS, and could not easily do so due to being > implemented entirely in C. > > This change fixes this by introducing a corresponding internal C > function reporting errors by return value and reimplementing the > `get-string-n!' in Scheme on top of that. Along with `get-string-n!', > `get-string-n' gets fixed, inheriting the correct behavior. > > * libguile/ports.c (scm_i_getc): New function, a version of `scm_getc' > not using exceptions. > (scm_getc): Implemented using `scm_i_getc'. > * libguile/ports.h (scm_i_getc): Add prototype marked SCM_INTERNAL. > > * libguile/r6rs-ports.c (scm_i_get_string_n_x): Exception-free version > of `get-string-n!', making use of `scm_i_getc'. > (scm_get_string_n_x): Removed, now implemented in Scheme. > > * module/ice-9/binary-ports.scm (get-string-n!): Removed from export > list, it doesn't fit the module module purpose anyway. > * module/rnrs/io/ports.scm (%get-string-n): Newly defined by internal > reference to `(ice-9 binary-ports)'. > (get-string-n!): Implemented in Scheme on top of `%get-string-n!'. > > * test-suite/tests/r6rs-ports.test ("8.2.9 Textual > input")["read-error"]: Activate commented-out exception-behavior tests > of `get-string-n!'. > ["decoding error"]: New test prefix with tests for `get-char', > `get-string-n!' and `get-string-n' and `get-line'. > --- > libguile/ports.c | 20 ++++++++++++++++---- > libguile/ports.h | 1 + > libguile/r6rs-ports.c | 21 +++++++++++---------- > module/ice-9/binary-ports.scm | 6 +++--- > module/rnrs/io/ports.scm | 14 ++++++++++++++ > test-suite/tests/r6rs-ports.test | 24 ++++++++++++++++++++---- > 6 files changed, 65 insertions(+), 21 deletions(-) > > diff --git a/libguile/ports.c b/libguile/ports.c > index 55808e2..b653af4 100644 > --- a/libguile/ports.c > +++ b/libguile/ports.c > @@ -1392,12 +1392,10 @@ scm_t_wchar > scm_getc (SCM port) > #define FUNC_NAME "scm_getc" > { > - int err; > - size_t len; > + int err = 0; > scm_t_wchar codepoint; > - char buf[SCM_MBCHAR_BUF_SIZE]; > > - err = get_codepoint (port, &codepoint, buf, &len); > + codepoint = scm_i_getc (port, &err); > if (SCM_UNLIKELY (err != 0)) > /* At this point PORT should point past the invalid encoding, as per > R6RS-lib Section 8.2.4. */ > @@ -1407,6 +1405,20 @@ scm_getc (SCM port) > } > #undef FUNC_NAME > > +/* Read a codepoint from PORT and return it. This version reports > + errors via the ERROR argument instead of via exceptions. */ > +scm_t_wchar > +scm_i_getc (SCM port, int *error) > +{ > + size_t len; > + scm_t_wchar codepoint; > + char buf[SCM_MBCHAR_BUF_SIZE]; > + > + *error = get_codepoint (port, &codepoint, buf, &len); > + > + return codepoint; > +} Given how trivial 'scm_i_getc' is, I think I'd prefer to leave 'scm_getc' alone, to avoid the additional overhead of another non-static C function call, which has to be done via the procedure linkage table (PLT) when libguile is a shared library and is thus not entirely trivial. > diff --git a/libguile/r6rs-ports.c b/libguile/r6rs-ports.c > index e867429..bd10081 100644 > --- a/libguile/r6rs-ports.c > +++ b/libguile/r6rs-ports.c > @@ -1242,18 +1242,17 @@ SCM_DEFINE (scm_i_make_transcoded_port, > > /* Textual I/O */ > > -SCM_DEFINE (scm_get_string_n_x, > - "get-string-n!", 4, 0, 0, > +SCM_DEFINE (scm_i_get_string_n_x, > + "%get-string-n!", 4, 0, 0, I'm a little bit nervous about this. Although it is not documented in the manual, 'scm_get_string_n_x' is declared in r6rs-ports.h as SCM_API. I'm not sure it's safe to make that go away. Why not leave the API as-is, and in the event of an error, just raise the proper R6RS exception from within 'scm_get_string_n_x'? > diff --git a/module/ice-9/binary-ports.scm b/module/ice-9/binary-ports.scm > index c07900b..3f7b9e6 100644 > --- a/module/ice-9/binary-ports.scm > +++ b/module/ice-9/binary-ports.scm > @@ -37,14 +37,14 @@ > get-bytevector-n! > get-bytevector-some > get-bytevector-all > - get-string-n! Users may have come to rely on this export from (ice-9 binary-ports). I don't think it's safe to remove it. Regards, Mark