[PATCH] Add private port structure, and move iconv descriptors there

[Mark H Weaver]

Hello all,

Here's a patch to add a private port structure.  I moved both input_cd
and output_cd there.  I plan to create more patches on top of this very
soon (BOM handling, more efficient per-port read options, maybe better
EOF handling), so a prompt review would be very helpful.

What do you think?

    Mark


>From 47aadc79423200b226d7592ea6bbeb788f1c5bef Mon Sep 17 00:00:00 2001
From: Mark H Weaver <address@hidden>
Date: Wed, 27 Mar 2013 15:41:45 -0400
Subject: [PATCH] Add private port structure, and move the iconv descriptors
 there.

* libguile/private-ports.h: New file.

* libguile/Makefile.am (noinst_HEADERS): Add private-ports.h.

* libguile/ports.h (scm_t_port): Replace 'input_cd' and 'output_cd'
  fields with 'internal' and 'reserved_for_future_use'.

* libguile/ports.c: Include private-ports.h.
  (finalize_port, scm_i_remove_port, get_iconv_codepoint): Access
  'input_cd' and 'output_cd' via 'internal' pointer.
  (scm_new_port_table_entry): Initialize new internal fields.
  Access 'input_cd' and 'output_cd' via 'internal' pointer.
  (get_codepoint): Use 'internal->encoding_type' to detect if the
  encoders are initialized and which encoding method to use.
  (scm_i_set_port_encoding_x): Set the 'internal->encoding_type' field
  appropriately.  Access 'input_cd' and 'output_cd' via 'internal'
  pointer.

* libguile/print.c: Include private-ports.h.
  (display_string_using_iconv): Access 'input_cd' and 'output_cd' via
  'internal' pointer.
  (display_string): Use 'internal->encoding_type' to detect if the
  encoders are initialized and which encoding method to use.
---
 libguile/Makefile.am     |    2 +-
 libguile/ports.c         |   79 +++++++++++++++++++++++++++++++---------------
 libguile/ports.h         |    7 ++--
 libguile/print.c         |   26 +++++++++------
 libguile/private-ports.h |   41 ++++++++++++++++++++++++
 5 files changed, 116 insertions(+), 39 deletions(-)
 create mode 100644 libguile/private-ports.h

diff --git a/libguile/Makefile.am b/libguile/Makefile.am
index 4b1f96b..6eaca76 100644
--- a/libguile/Makefile.am
+++ b/libguile/Makefile.am
@@ -455,7 +455,7 @@ noinst_HEADERS = conv-integer.i.c conv-uinteger.i.c         
\
                  srfi-14.i.c                                   \
                  quicksort.i.c                                  \
                  win32-uname.h                                 \
-                private-gc.h private-options.h
+                private-gc.h private-options.h private-ports.h
 
 # vm instructions
 noinst_HEADERS += vm-engine.c vm-i-system.c vm-i-scheme.c vm-i-loader.c
diff --git a/libguile/ports.c b/libguile/ports.c
index 8737a76..45e770d 100644
--- a/libguile/ports.c
+++ b/libguile/ports.c
@@ -60,6 +60,8 @@
 #include "libguile/fluids.h"
 #include "libguile/eq.h"
 
+#include "libguile/private-ports.h"
+
 #ifdef HAVE_STRING_H
 #include <string.h>
 #endif
@@ -589,10 +591,10 @@ finalize_port (void *ptr, void *data)
 
          entry = SCM_PTAB_ENTRY (port);
 
-         if (entry->input_cd != (iconv_t) -1)
-           iconv_close (entry->input_cd);
-         if (entry->output_cd != (iconv_t) -1)
-           iconv_close (entry->output_cd);
+         if (entry->internal->input_cd != (iconv_t) -1)
+           iconv_close (entry->internal->input_cd);
+         if (entry->internal->output_cd != (iconv_t) -1)
+           iconv_close (entry->internal->output_cd);
 
          SCM_SETSTREAM (port, 0);
          SCM_CLR_PORT_OPEN_FLAG (port);
@@ -629,9 +631,15 @@ scm_new_port_table_entry (scm_t_bits tag)
   enc = scm_i_default_port_encoding ();
   entry->encoding = enc ? scm_gc_strdup (enc, "port") : NULL;
 
+  /* Initialize private fields.  */
+  entry->reserved_for_future_use = NULL;
+  entry->internal = (struct scm_t_port_private *)
+    scm_gc_calloc (sizeof (struct scm_t_port_private), "port-private");
+  entry->internal->encoding_type = SCM_PORT_ENCODING_TYPE_UNINITIALIZED;
+
   /* The conversion descriptors will be opened lazily.  */
-  entry->input_cd = (iconv_t) -1;
-  entry->output_cd = (iconv_t) -1;
+  entry->internal->input_cd = (iconv_t) -1;
+  entry->internal->output_cd = (iconv_t) -1;
 
   entry->ilseq_handler = scm_i_default_port_conversion_handler ();
 
@@ -686,18 +694,20 @@ scm_i_remove_port (SCM port)
   p->putback_buf = NULL;
   p->putback_buf_size = 0;
 
-  if (p->input_cd != (iconv_t) -1)
+  if (p->internal->input_cd != (iconv_t) -1)
     {
-      iconv_close (p->input_cd);
-      p->input_cd = (iconv_t) -1;
+      iconv_close (p->internal->input_cd);
+      p->internal->input_cd = (iconv_t) -1;
     }
   
-  if (p->output_cd != (iconv_t) -1)
+  if (p->internal->output_cd != (iconv_t) -1)
     {
-      iconv_close (p->output_cd);
-      p->output_cd = (iconv_t) -1;
+      iconv_close (p->internal->output_cd);
+      p->internal->output_cd = (iconv_t) -1;
     }
 
+  p->internal = NULL;
+
   SCM_SETPTAB_ENTRY (port, 0);
 
   scm_hashq_remove_x (scm_i_port_weak_hash, port);
@@ -1297,12 +1307,14 @@ get_iconv_codepoint (SCM port, scm_t_wchar *codepoint,
                     char buf[SCM_MBCHAR_BUF_SIZE], size_t *len)
 {
   scm_t_port *pt;
+  struct scm_t_port_private *pti;
   int err, byte_read;
   size_t bytes_consumed, output_size;
   char *output;
   scm_t_uint8 utf8_buf[SCM_MBCHAR_BUF_SIZE];
 
   pt = SCM_PTAB_ENTRY (port);
+  pti = pt->internal;
 
   for (output_size = 0, output = (char *) utf8_buf,
         bytes_consumed = 0, err = 0;
@@ -1332,7 +1344,7 @@ get_iconv_codepoint (SCM port, scm_t_wchar *codepoint,
       input_left = bytes_consumed + 1;
       output_left = sizeof (utf8_buf);
 
-      done = iconv (pt->input_cd, &input, &input_left,
+      done = iconv (pti->input_cd, &input, &input_left,
                    &output, &output_left);
       if (done == (size_t) -1)
        {
@@ -1369,15 +1381,22 @@ get_codepoint (SCM port, scm_t_wchar *codepoint,
   int err;
   scm_t_port *pt = SCM_PTAB_ENTRY (port);
 
-  if (pt->input_cd == (iconv_t) -1)
+  if (pt->internal->encoding_type == SCM_PORT_ENCODING_TYPE_UNINITIALIZED)
     /* Initialize the conversion descriptors, if needed.  */
     scm_i_set_port_encoding_x (port, pt->encoding);
 
-  /* FIXME: In 2.1, add a flag to determine whether a port is UTF-8.  */
-  if (pt->input_cd == (iconv_t) -1)
-    err = get_utf8_codepoint (port, codepoint, (scm_t_uint8 *) buf, len);
-  else
-    err = get_iconv_codepoint (port, codepoint, buf, len);
+  switch (pt->internal->encoding_type)
+    {
+    case SCM_PORT_ENCODING_TYPE_ICONV:
+      err = get_iconv_codepoint (port, codepoint, buf, len);
+      break;
+    case SCM_PORT_ENCODING_TYPE_UTF8:
+      err = get_utf8_codepoint (port, codepoint, (scm_t_uint8 *) buf, len);
+      break;
+    case SCM_PORT_ENCODING_TYPE_UNINITIALIZED:
+    default:
+      scm_syserror ("get_codepoint");
+    }
 
   if (SCM_LIKELY (err == 0))
     update_port_lf (*codepoint, port);
@@ -2208,13 +2227,16 @@ void
 scm_i_set_port_encoding_x (SCM port, const char *encoding)
 {
   scm_t_port *pt;
+  struct scm_t_port_private *pti;
   iconv_t new_input_cd, new_output_cd;
+  enum scm_t_port_encoding_type new_encoding_type;
 
   new_input_cd = (iconv_t) -1;
   new_output_cd = (iconv_t) -1;
 
   /* Set the character encoding for this port.  */
   pt = SCM_PTAB_ENTRY (port);
+  pti = pt->internal;
 
   if (encoding == NULL)
     encoding = "ISO-8859-1";
@@ -2225,8 +2247,12 @@ scm_i_set_port_encoding_x (SCM port, const char 
*encoding)
   /* If ENCODING is UTF-8, then no conversion descriptor is opened
      because we do I/O ourselves.  This saves 100+ KiB for each
      descriptor.  */
-  if (strcmp (encoding, "UTF-8"))
+  if (!strcmp (encoding, "UTF-8"))
+    new_encoding_type = SCM_PORT_ENCODING_TYPE_UTF8;
+  else
     {
+      new_encoding_type = SCM_PORT_ENCODING_TYPE_ICONV;
+
       if (SCM_CELL_WORD_0 (port) & SCM_RDNG)
        {
          /* Open an input iconv conversion descriptor, from ENCODING
@@ -2251,13 +2277,14 @@ scm_i_set_port_encoding_x (SCM port, const char 
*encoding)
        }
     }
 
-  if (pt->input_cd != (iconv_t) -1)
-    iconv_close (pt->input_cd);
-  if (pt->output_cd != (iconv_t) -1)
-    iconv_close (pt->output_cd);
+  if (pti->input_cd != (iconv_t) -1)
+    iconv_close (pti->input_cd);
+  if (pti->output_cd != (iconv_t) -1)
+    iconv_close (pti->output_cd);
 
-  pt->input_cd = new_input_cd;
-  pt->output_cd = new_output_cd;
+  pti->input_cd = new_input_cd;
+  pti->output_cd = new_output_cd;
+  pti->encoding_type = new_encoding_type;
 
   return;
 
diff --git a/libguile/ports.h b/libguile/ports.h
index d4d59b7..8aa29ee 100644
--- a/libguile/ports.h
+++ b/libguile/ports.h
@@ -43,6 +43,8 @@ typedef enum scm_t_port_rw_active {
   SCM_PORT_WRITE = 2
 } scm_t_port_rw_active;
 
+struct scm_t_port_private;
+
 /* C representation of a Scheme port.  */
 
 typedef struct 
@@ -112,9 +114,8 @@ typedef struct
   unsigned char *putback_buf;
   size_t putback_buf_size;        /* allocated size of putback_buf.  */
 
-  /* input/output iconv conversion descriptors */
-  void *input_cd;
-  void *output_cd;
+  struct scm_t_port_private *internal;
+  void *reserved_for_future_use;
 } scm_t_port;
 
 
diff --git a/libguile/print.c b/libguile/print.c
index 647eed8..51f9f01 100644
--- a/libguile/print.c
+++ b/libguile/print.c
@@ -56,6 +56,7 @@
 #include "libguile/print.h"
 
 #include "libguile/private-options.h"
+#include "libguile/private-ports.h"
 
 
 
@@ -880,8 +881,10 @@ display_string_using_iconv (const void *str, int narrow_p, 
size_t len,
 {
   size_t printed;
   scm_t_port *pt;
+  struct scm_t_port_private *pti;
 
   pt = SCM_PTAB_ENTRY (port);
+  pti = pt->internal;
 
   printed = 0;
 
@@ -910,7 +913,7 @@ display_string_using_iconv (const void *str, int narrow_p, 
size_t len,
       output = encoded_output;
       output_left = sizeof (encoded_output);
 
-      done = iconv (pt->output_cd, &input, &input_left,
+      done = iconv (pti->output_cd, &input, &input_left,
                    &output, &output_left);
 
       output_len = sizeof (encoded_output) - output_left;
@@ -920,7 +923,7 @@ display_string_using_iconv (const void *str, int narrow_p, 
size_t len,
           int errno_save = errno;
 
          /* Reset the `iconv' state.  */
-         iconv (pt->output_cd, NULL, NULL, NULL, NULL);
+         iconv (pti->output_cd, NULL, NULL, NULL, NULL);
 
          /* Print the OUTPUT_LEN bytes successfully converted.  */
          scm_lfwrite (encoded_output, output_len, port);
@@ -984,16 +987,21 @@ display_string (const void *str, int narrow_p,
 
   pt = SCM_PTAB_ENTRY (port);
 
-  if (pt->output_cd == (iconv_t) -1)
+  if (pt->internal->encoding_type == SCM_PORT_ENCODING_TYPE_UNINITIALIZED)
     /* Initialize the conversion descriptors, if needed.  */
     scm_i_set_port_encoding_x (port, pt->encoding);
 
-  /* FIXME: In 2.1, add a flag to determine whether a port is UTF-8.  */
-  if (pt->output_cd == (iconv_t) -1)
-    return display_string_as_utf8 (str, narrow_p, len, port);
-  else
-    return display_string_using_iconv (str, narrow_p, len,
-                                      port, strategy);
+  switch (pt->internal->encoding_type)
+    {
+    case SCM_PORT_ENCODING_TYPE_ICONV:
+      return display_string_using_iconv (str, narrow_p, len,
+                                         port, strategy);
+    case SCM_PORT_ENCODING_TYPE_UTF8:
+      return display_string_as_utf8 (str, narrow_p, len, port);
+    case SCM_PORT_ENCODING_TYPE_UNINITIALIZED:
+    default:
+      scm_syserror ("display_string");
+    }
 }
 
 /* Attempt to display CH to PORT according to STRATEGY.  Return non-zero
diff --git a/libguile/private-ports.h b/libguile/private-ports.h
new file mode 100644
index 0000000..369c4a5
--- /dev/null
+++ b/libguile/private-ports.h
@@ -0,0 +1,41 @@
+/*
+ * private-ports.h - private declarations for ports.
+ * 
+ * Copyright (C) 2013 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
+ * as published by the Free Software Foundation; either version 3 of
+ * the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ */
+
+#ifndef SCM_PRIVATE_PORTS
+#define SCM_PRIVATE_PORTS
+
+#include "libguile/_scm.h"
+#include "libguile/ports.h"
+
+enum scm_t_port_encoding_type {
+  SCM_PORT_ENCODING_TYPE_UNINITIALIZED = 0,
+  SCM_PORT_ENCODING_TYPE_ICONV = 1,
+  SCM_PORT_ENCODING_TYPE_UTF8 = 2
+};
+
+struct scm_t_port_private
+{
+  enum scm_t_port_encoding_type encoding_type;
+  void *input_cd;
+  void *output_cd;
+};
+
+#endif
-- 
1.7.10.4