From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Andreas Rottmann Newsgroups: gmane.lisp.guile.devel Subject: Re: [PATCH] Fix `get-string-n!' &i/o-decoding exception behavior Date: Sun, 11 Nov 2012 19:31:23 +0100 Message-ID: <87haow8004.fsf@delenn.home.rotty.xx.vu> References: <1352332308-5191-1-git-send-email-a.rottmann@gmx.at> <874nkwa9bw.fsf@tines.lan> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: ger.gmane.org 1352659119 24884 80.91.229.3 (11 Nov 2012 18:38:39 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sun, 11 Nov 2012 18:38:39 +0000 (UTC) Cc: guile-devel@gnu.org To: Mark H Weaver Original-X-From: guile-devel-bounces+guile-devel=m.gmane.org@gnu.org Sun Nov 11 19:38:49 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 1TXcQh-0004hl-5E for guile-devel@m.gmane.org; Sun, 11 Nov 2012 19:38:47 +0100 Original-Received: from localhost ([::1]:56510 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TXcQX-0001u9-OY for guile-devel@m.gmane.org; Sun, 11 Nov 2012 13:38:37 -0500 Original-Received: from eggs.gnu.org ([208.118.235.92]:59341) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TXcQS-0001tf-PX for guile-devel@gnu.org; Sun, 11 Nov 2012 13:38:35 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TXcQP-0000lD-NP for guile-devel@gnu.org; Sun, 11 Nov 2012 13:38:32 -0500 Original-Received: from mailout-de.gmx.net ([213.165.64.22]:34711) by eggs.gnu.org with smtp (Exim 4.71) (envelope-from ) id 1TXcQP-0000kh-EQ for guile-devel@gnu.org; Sun, 11 Nov 2012 13:38:29 -0500 Original-Received: (qmail invoked by alias); 11 Nov 2012 18:31:32 -0000 Original-Received: from 91-119-177-155.dynamic.xdsl-line.inode.at (EHLO cubox.home.rotty.xx.vu) [91.119.177.155] by mail.gmx.net (mp004) with SMTP; 11 Nov 2012 19:31:32 +0100 X-Authenticated: #3102804 X-Provags-ID: V01U2FsdGVkX1+TFE8AsxWT0C/80R8bFqRKr5AuY5Fy91NSVIEim5 zCj4TohsX7lCZI Original-Received: from delenn.home.rotty.xx.vu (unknown [192.168.2.11]) by cubox.home.rotty.xx.vu (Postfix) with ESMTP id EDD93160087; Sun, 11 Nov 2012 19:31:23 +0100 (CET) Original-Received: by delenn.home.rotty.xx.vu (Postfix, from userid 1000) id AD33832002C; Sun, 11 Nov 2012 19:31:23 +0100 (CET) In-Reply-To: <874nkwa9bw.fsf@tines.lan> (Mark H. Weaver's message of "Sun, 11 Nov 2012 02:26:59 -0500") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3.50 (gnu/linux) X-Y-GMX-Trusted: 0 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x [generic] X-Received-From: 213.165.64.22 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:15154 Archived-At: Mark H Weaver writes: > Hi Andreas, > > Thanks for the patch. See below for my comments. > > Andreas Rottmann writes: > >> 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. > Yup, that's fine with me. >> 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. > Oh, I missed that one; thanks. > 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'? > The problem here is that we have no easy way to raise R6RS exceptions from C code, AFAICT. It is certainly possible, but if it involves convoluted code of doing imports of condition types and appropriate constructors, then constructing a proper invocation, all in C, I'd rather avoid it. I think the tendency (in general, also in Guile's implementation) was to do more things in Scheme, and less in C. However, exceptions are a difficult topic; if we want efficient (i.e. ones not requiring setting up exception converters on each call) implementations of R6RS I/O procedures eventually, we'd either (a) need to expose exception-less primitives (like I attempted with scm_i_get_c and scm_i_get_string_n_x), and use those to implement the actual exception-throwing procedures in Scheme, or (b) if you really want this done (or doable) fully in C, I think we'd first provide an API (at least an internal one), to make it possible to easily raise R6RS conditions from C. In that sense, it's unfortunate that r6rs-ports.h is public API at all, since it effectively prevents us from using strategy (a), at least without going through a deprecation phase. These functions are now all broken wrt. exceptions -- do we want to fix this at the C level as well, or rather deprecate them, and DTRT (only) for their Scheme counterparts? >> 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. > Hmm, yeah, that has bothered me a bit as well, but I forgot to explicitly point it out making the patch potentially unsuitable for stable. Nevertheless, having textual I/O procedures in `(ice-9 binary-ports)' seems quite a bit strange; maybe leave it there (for stable-2.0) but mark that binding as deprecated -- if users want R6RS textual I/O, they know where to find it ;-). Also, I'm not sure if procedures in `(ice-9 ...)' can be expected to be throw R6RS conditions at all... Regards, Rotty -- Andreas Rottmann --