Re: iconv_alloc for iconvme module

[Simon Josefsson]

"Oskar Liljeblad" <address@hidden> writes:

> Here's a patch adding a function iconv_alloc to the iconvme module.
> It differs from the function iconv_string in that it takes a previously
> opened iconv descriptor instead of the explicit codeset names.

Thanks!  I installed this, after fixing two nits:

> +  {
> +    int save_errno = errno;
> +
> +    if (iconv_close (cd) < 0 && dest)
> +      {
> +     /* If we didn't have a real error before, make sure we restore
> +        the iconv_close error below. */
> +     free (dest);
> +     dest = NULL;
> +      }
> +    else
> +      errno = save_errno;
> +  }

You need to restore the errno set by iconv_close after the call to
free.

>  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
> +  if (have_error && dest)
> +    {
> +      free(dest);
> +      dest = NULL;
> +    }
>  
>    return dest;

Same here, free may corrupt the errno from iconv (or others) that
should be returned.

In the future, it will be easier to install if you suggest ChangeLog
entries.

Regards,
Simon