unofficial mirror of guile-devel@gnu.org 
 help / color / mirror / Atom feed
From: Mike Gran <spk121@yahoo.com>
To: "Ludovic Courtès" <ludo@gnu.org>, guile-devel@gnu.org
Subject: Re: [PATCH] Have string ports honor ‘%default-port-encoding’
Date: Mon, 4 Jan 2010 18:47:47 -0800 (PST)	[thread overview]
Message-ID: <217394.1649.qm@web37902.mail.mud.yahoo.com> (raw)
In-Reply-To: <87pr5p9zg0.fsf@gnu.org>

> From: Ludovic Courtès <ludo@gnu.org>
> 
> Hello,
> 
> The attached patch fixes string ports so that their encoding defaults to
> ‘%default-port-encoding’ (as for other ports) instead of UTF-8.
> 
> Mike: can you review it?

Hi Ludovic, 

It is a bit hard for me to review this as I don't have access to my machine
right now, but, let me see what I can do.

When you write...

+  /* Create a copy of STR in the encoding of Z.  */
+  buf = scm_to_stringn (str, &str_len, pt->encoding,
+            SCM_FAILED_CONVERSION_ERROR);
+  /* FIXME: strdup doesn't do the right thing if BUF contains zeros, but we
+    don't know the size in bytes of STR.  */
+  c_str = scm_gc_strdup (buf, "strport");
+  free (buf);

... isn't the returned value str_len the length in bytes of buf?  I think
you could avoid the strdup call, since it could fail, for example, for
UTF-32 strings of more than one character.

Also, in the big scheme of things, I wonder if the name "string port"
is misleading now.  Strings can contain the whole codepoint range.  
But string ports can't store the whole range depending on their encoding.
(That's what the "UTF-8" hack was about.)

Thanks,

Mike Gran




  reply	other threads:[~2010-01-05  2:47 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-01-05  0:18 [PATCH] Have string ports honor ‘%default-port-encoding’ Ludovic Courtès
2010-01-05  2:47 ` Mike Gran [this message]
2010-01-05  9:14   ` Ludovic Courtès
2010-01-05 18:21     ` Mike Gran
2010-01-07 10:37       ` Ludovic Courtès

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/guile/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=217394.1649.qm@web37902.mail.mud.yahoo.com \
    --to=spk121@yahoo.com \
    --cc=guile-devel@gnu.org \
    --cc=ludo@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).