[bug-gnulib] Re: iconv made easy

[Paul Eggert]

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.

>       if (!have_error)
>         save_errno = errno;
>       have_error = 1;

A minor point.  That could be

  if (!have_error)
    {
      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.

> 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.