bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[bug-gnulib] Re: iconv made easy


From: Simon Josefsson
Subject: [bug-gnulib] Re: iconv made easy
Date: Tue, 28 Dec 2004 21:34:19 +0100
User-agent: Gnus/5.110003 (No Gnus v0.3) Emacs/21.3.50 (gnu/linux)

Paul Eggert <address@hidden> writes:

> Simon Josefsson <address@hidden> writes:
>
>>> If newsize <= outbuf_size, this sets have_error=1 and the remaining
>>> code eventually uses errno.  But errno is garbage at that point.  You
>>> need to set errno to ENOMEM in that case.
>>
>> No, I believe errno would have been E2BIG if 'newsize <= outbuf_size'
>> trigger the if case.  But I have changed it into:
>>
>>          if (newsize <= outbuf_size)
>>            {
>>              errno = EOVERFLOW;
>
> Sorry, I did't realize it was E2BIG.  But it should be set to ENOMEM.
> That is what (for example) GNU calloc does on size_t overflow.

Done.

>>      if (!have_error)
>>        save_errno = errno;
>>      have_error = 1;
>
> A minor point.  That could be
>
>   if (!have_error)
>     {
>       save_errno = errno;
>       have_error = 1;
>     }

Or even:

    if (iconv_close (cd) < 0 && !have_error)
      {
        /* If we didn't have a real error before, make sure we restore
           the iconv_close error below. */
        save_errno = errno;
        have_error = 1;
      }

> Perhaps you should rewrite it to remove have_error, and consistently
> use the test "save_errno == 0" instead of "!have_error".  That will
> simplify the code a bit.

Is that reliable?  I though that errno could be left alone (i.e., left
as garbage) when a system function succeeds.

OTOH, the function start out by doing 'errno = 0'.

Generally, aren't these errno assignments unportable?  I recall that
errno was only guaranteed to be a lvalue, but not a rvalue.  (I.e., it
could be a macro.)

I could not find anything about errno in README.

I do agree that the code is difficult to follow, though.  I think it
would improve the code if the main switch case was moved to another
function.  But it might not be worth all the trouble, if Bruno will
replace this implementation with his more generic stuff soonish.

>> The POSIX prototype uses 'char **restrict'.  I can't find any text
>> that say the function doesn't modify inbuf.
>
> Sorry, I got turned around then.  Most likely some nonstandard
> implementations use char * const *, then.  Anyway, it shouldn't
> modify inbuf, and I wouldn't slow down the implementation to worry
> about that possibility.
>
>> +  ICONV_CONST char *p = (ICONV_CONST char *) str;
>
> And this suggests that you should simply write this:
>
>    char *p = (char *) str;
>
> as this will work with both the POSIX and the non-POSIX version.

Done.

Index: iconvme.c
===================================================================
RCS file: iconvme.c
diff -N iconvme.c
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ iconvme.c   28 Dec 2004 20:32:45 -0000
@@ -0,0 +1,154 @@
+/* Recode strings between character sets, using iconv.
+   Copyright (C) 2002, 2003, 2004 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or
+   modify it under the terms of the GNU General Public License as
+   published by the Free Software Foundation; either version 2, or (at
+   your option) any later version.
+
+   This program 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 General Public License for more details.
+
+   You should have received a copy of the GNU General Public License along
+   with this program; if not, write to the Free Software Foundation,
+   Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  */
+
+#ifdef HAVE_CONFIG_H
+# include <config.h>
+#endif
+
+/* Get prototype. */
+#include "iconvme.h"
+
+/* Get malloc. */
+#include <stdlib.h>
+
+/* Get strcmp. */
+#include <string.h>
+
+/* Get errno. */
+#include <errno.h>
+
+#if HAVE_ICONV
+/* Get iconv etc. */
+# include <iconv.h>
+/* Get MB_LEN_MAX. */
+# include <limits.h>
+#endif
+
+/* Get strdup. */
+#include "strdup.h"
+
+/* Convert a zero-terminated string STR from the FROM_CODSET code set
+   to the TO_CODESET code set.  The returned string is allocated using
+   malloc, and must be dellocated by the caller using free.  On
+   failure, NULL is returned and errno holds the error reason.  Note
+   that if TO_CODESET uses \0 for anything but to terminate the
+   string, the caller of this function may have difficulties finding
+   out the length of the output string.  */
+char *
+iconv_string (const char *str, const char *from_codeset,
+             const char *to_codeset)
+{
+  char *dest = NULL;
+#if HAVE_ICONV
+  iconv_t cd;
+  char *outp;
+  char *p = (char *) str;
+  size_t inbytes_remaining = strlen (p);
+  /* Guess the maximum length the output string can have.  */
+  size_t outbuf_size = (inbytes_remaining + 1) * MB_LEN_MAX;
+  size_t outbytes_remaining = outbuf_size - 1; /* -1 for NUL */
+  size_t err;
+  int have_error = 0;
+#endif
+
+  if (strcmp (to_codeset, from_codeset) == 0)
+    return strdup (str);
+
+#if HAVE_ICONV
+  cd = iconv_open (to_codeset, from_codeset);
+  if (cd == (iconv_t) - 1)
+    return NULL;
+
+  outp = dest = malloc (outbuf_size);
+  if (dest == NULL)
+    goto out;
+
+again:
+  err = iconv (cd, &p, &inbytes_remaining, &outp, &outbytes_remaining);
+
+  if (err == (size_t) - 1)
+    {
+      switch (errno)
+       {
+       case EINVAL:
+         /* Incomplete text, do not report an error */
+         break;
+
+       case E2BIG:
+         {
+           size_t used = outp - dest;
+           size_t newsize = outbuf_size * 2;
+           char *newdest;
+
+           if (newsize <= outbuf_size)
+             {
+               errno = ENOMEM;
+               have_error = 1;
+               goto out;
+             }
+           if (!(newdest = realloc (dest, newsize)))
+             {
+               have_error = 1;
+               goto out;
+             }
+           dest = newdest;
+           outbuf_size = newsize;
+
+           outp = dest + used;
+           outbytes_remaining = outbuf_size - used - 1;        /* -1 for NUL */
+
+           goto again;
+         }
+         break;
+
+       case EILSEQ:
+         have_error = 1;
+         break;
+
+       default:
+         have_error = 1;
+         break;
+       }
+    }
+
+  *outp = '\0';
+
+out:
+  {
+    int save_errno = errno;
+
+    if (iconv_close (cd) < 0 && !have_error)
+      {
+       /* If we didn't have a real error before, make sure we restore
+          the iconv_close error below. */
+       save_errno = errno;
+       have_error = 1;
+      }
+
+    if (have_error && dest)
+      {
+       free (dest);
+       dest = NULL;
+       errno = save_errno;
+      }
+  }
+#else
+  errno = ENOSYS;
+#endif
+
+  return dest;
+}




reply via email to

[Prev in Thread] Current Thread [Next in Thread]