Let mutt_ch_choose report conversion failure (#1137)

* Let mutt_ch_choose report conversion failure

Issue #1136

* Let mutt_ch_convert_string return errno if iconv(..) fails

Previously, mutt_ch_convert_string returned error only if any arguments
failed the sanity check or iconv_open() failed.

With this change, an error value is returned even if the iconv()
conversion fails, such as when the input string contains an invalid
character in the source character set (EILSEQ).

I have looked at each caller and it seems to me that this change won't
introduce any misbehaviour, but I'm far from being 100% confident.

Issue #1136

* test int functions against 0

* Better error reporting in mutt_ch_iconv

Issue #1136

diff --git a/handler.c b/handler.c
index 7c51d91a012f5e42296d6397b0d886eb12cb3d7d..ad4b877097684f4892bc62dafc5a58b5e58febf1 100644 (file)
--- a/handler.c
+++ b/handler.c
@@ -102,7 +102,7 @@ static void convert_to_state(iconv_t cd, char *bufi, size_t *l, struct State *s)
   while (true)
   {
     ob = bufo, obl = sizeof(bufo);
-    mutt_ch_iconv(cd, &ib, &ibl, &ob, &obl, 0, "?");
+    mutt_ch_iconv(cd, &ib, &ibl, &ob, &obl, 0, "?", NULL);
     if (ob == bufo)
       break;
     state_prefix_put(bufo, ob - bufo, s);

diff --git a/imap/utf7.c b/imap/utf7.c
index 7c3bfa9b9db800a56f21cf95b7609c2d6c984795..a7d9854c353c247e3bcc2cd4020db67d68768ce1 100644 (file)
--- a/imap/utf7.c
+++ b/imap/utf7.c
@@ -320,7 +320,7 @@ void imap_utf_encode(struct ImapData *idata, char **s)
   if (Charset)
   {
     char *t = mutt_str_strdup(*s);
-    if (t && !mutt_ch_convert_string(&t, Charset, "utf-8", 0))
+    if (t && mutt_ch_convert_string(&t, Charset, "utf-8", 0) == 0)
     {
       FREE(s);
       if (idata->unicode)
@@ -348,7 +348,7 @@ void imap_utf_decode(struct ImapData *idata, char **s)
     else
       t = utf7_to_utf8(*s, strlen(*s), 0, 0);
 
-    if (t && !mutt_ch_convert_string(&t, "utf-8", Charset, 0))
+    if (t && mutt_ch_convert_string(&t, "utf-8", Charset, 0) == 0)
     {
       FREE(s);
       *s = t;

diff --git a/mutt/charset.c b/mutt/charset.c
index 26635ae8656e53fff01fa0ee09711b7ae06b8180..6ecb7e16304d8385031cdb26a560dc33b4fde0fd 100644 (file)
--- a/mutt/charset.c
+++ b/mutt/charset.c
@@ -564,6 +564,7 @@ iconv_t mutt_ch_iconv_open(const char *tocode, const char *fromcode, int flags)
  * @param[in,out] outbytesleft Length of result buffer
  * @param[in]     inrepls      Input replacement characters
  * @param[in]     outrepl      Output replacement characters
+ * @param[out]    iconverrno   Errno if iconv() fails, 0 if it succeeds
  * @retval num Number of characters converted
  *
  * Like iconv, but keeps going even when the input is invalid
@@ -571,7 +572,8 @@ iconv_t mutt_ch_iconv_open(const char *tocode, const char *fromcode, int flags)
  * if you're supplying an outrepl, the target charset should be.
  */
 size_t mutt_ch_iconv(iconv_t cd, const char **inbuf, size_t *inbytesleft, char **outbuf,
-                     size_t *outbytesleft, const char **inrepls, const char *outrepl)
+                     size_t *outbytesleft, const char **inrepls, const char *outrepl,
+                     int *iconverrno)
 {
   size_t rc = 0;
   const char *ib = *inbuf;
@@ -581,9 +583,13 @@ size_t mutt_ch_iconv(iconv_t cd, const char **inbuf, size_t *inbytesleft, char *
 
   while (true)
   {
+    errno = 0;
     const size_t ret1 = iconv(cd, (ICONV_CONST char **) &ib, &ibl, &ob, &obl);
     if (ret1 != (size_t) -1)
       rc += ret1;
+    if (iconverrno)
+      *iconverrno = errno;
+
     if (ibl && obl && errno == EILSEQ)
     {
       if (inrepls)
@@ -660,8 +666,9 @@ const char *mutt_ch_iconv_lookup(const char *chs)
  * @param[in]     from  Current character set
  * @param[in]     to    Target character set
  * @param[in]     flags Flags, e.g.
- * @retval 0  Success
- * @retval -1 Error
+ * @retval 0      Success
+ * @retval -1     Invalid arguments or failure to open an iconv channel
+ * @retval errno  Failure in iconv conversion
  *
  * Parameter flags is given as-is to mutt_ch_iconv_open().
  * See there for its meaning and usage policy.
@@ -671,45 +678,48 @@ int mutt_ch_convert_string(char **ps, const char *from, const char *to, int flag
   iconv_t cd;
   const char *repls[] = { "\357\277\275", "?", 0 };
   char *s = *ps;
+  int rc = 0;
 
   if (!s || !*s)
     return 0;
 
-  if (to && from && (cd = mutt_ch_iconv_open(to, from, flags)) != (iconv_t) -1)
-  {
-    size_t len;
-    const char *ib = NULL;
-    char *buf = NULL, *ob = NULL;
-    size_t ibl, obl;
-    const char **inrepls = NULL;
-    char *outrepl = NULL;
-
-    if (mutt_ch_is_utf8(to))
-      outrepl = "\357\277\275";
-    else if (mutt_ch_is_utf8(from))
-      inrepls = repls;
-    else
-      outrepl = "?";
+  if (!to || !from)
+    return -1;
 
-    len = strlen(s);
-    ib = s;
-    ibl = len + 1;
-    obl = MB_LEN_MAX * ibl;
-    ob = buf = mutt_mem_malloc(obl + 1);
+  cd = mutt_ch_iconv_open(to, from, flags);
+  if (cd == (iconv_t) -1)
+    return -1;
 
-    mutt_ch_iconv(cd, &ib, &ibl, &ob, &obl, inrepls, outrepl);
-    iconv_close(cd);
+  size_t len;
+  const char *ib = NULL;
+  char *buf = NULL, *ob = NULL;
+  size_t ibl, obl;
+  const char **inrepls = NULL;
+  char *outrepl = NULL;
+
+  if (mutt_ch_is_utf8(to))
+    outrepl = "\357\277\275";
+  else if (mutt_ch_is_utf8(from))
+    inrepls = repls;
+  else
+    outrepl = "?";
 
-    *ob = '\0';
+  len = strlen(s);
+  ib = s;
+  ibl = len + 1;
+  obl = MB_LEN_MAX * ibl;
+  ob = buf = mutt_mem_malloc(obl + 1);
 
-    FREE(ps);
-    *ps = buf;
+  mutt_ch_iconv(cd, &ib, &ibl, &ob, &obl, inrepls, outrepl, &rc);
+  iconv_close(cd);
 
-    mutt_str_adjust(ps);
-    return 0;
-  }
-  else
-    return -1;
+  *ob = '\0';
+
+  FREE(ps);
+  *ps = buf;
+
+  mutt_str_adjust(ps);
+  return rc;
 }
 
 /**
@@ -848,7 +858,7 @@ int mutt_ch_fgetconv(struct FgetConv *fc)
   if (fc->ibl)
   {
     size_t obl = sizeof(fc->bufo);
-    mutt_ch_iconv(fc->cd, (const char **) &fc->ib, &fc->ibl, &fc->ob, &obl, fc->inrepls, 0);
+    mutt_ch_iconv(fc->cd, (const char **) &fc->ib, &fc->ibl, &fc->ob, &obl, fc->inrepls, 0, NULL);
     if (fc->p < fc->ob)
       return (unsigned char) *(fc->p)++;
   }
@@ -954,7 +964,7 @@ char *mutt_ch_choose(const char *fromcode, const char *charsets, char *u,
     t[n] = '\0';
 
     s = mutt_str_substr_dup(u, u + ulen);
-    if (mutt_ch_convert_string(&s, fromcode, t, 0))
+    if (mutt_ch_convert_string(&s, fromcode, t, 0) != 0)
     {
       FREE(&t);
       FREE(&s);

diff --git a/mutt/charset.h b/mutt/charset.h
index 1c585d19e424c7871f265e27e1c035c62125a0ce..2597d4c4f450174943c6b8cf00a160f9f1f3a7f8 100644 (file)
--- a/mutt/charset.h
+++ b/mutt/charset.h
@@ -93,7 +93,7 @@ void             mutt_ch_lookup_remove(void);
 const char *     mutt_ch_charset_lookup(const char *chs);
 
 iconv_t          mutt_ch_iconv_open(const char *tocode, const char *fromcode, int flags);
-size_t           mutt_ch_iconv(iconv_t cd, const char **inbuf, size_t *inbytesleft, char **outbuf, size_t *outbytesleft, const char **inrepls, const char *outrepl);
+size_t           mutt_ch_iconv(iconv_t cd, const char **inbuf, size_t *inbytesleft, char **outbuf, size_t *outbytesleft, const char **inrepls, const char *outrepl, int *iconverrno);
 const char *     mutt_ch_iconv_lookup(const char *chs);
 int              mutt_ch_convert_string(char **ps, const char *from, const char *to, int flags);
 int              mutt_ch_convert_nonmime_string(char **ps);

diff --git a/mutt/idna.c b/mutt/idna.c
index b8fdb16465eb4b6506c65c08ca1945c0e677188e..2ecf680a7b01a8c2de5240226c50e697e93eebaa 100644 (file)
--- a/mutt/idna.c
+++ b/mutt/idna.c
@@ -142,10 +142,10 @@ char *mutt_idna_intl_to_local(const char *user, const char *domain, int flags)
 #endif /* HAVE_LIBIDN */
 
   /* we don't want charset-hook effects, so we set flags to 0 */
-  if (mutt_ch_convert_string(&local_user, "utf-8", Charset, 0) == -1)
+  if (mutt_ch_convert_string(&local_user, "utf-8", Charset, 0) != 0)
     goto cleanup;
 
-  if (mutt_ch_convert_string(&local_domain, "utf-8", Charset, 0) == -1)
+  if (mutt_ch_convert_string(&local_domain, "utf-8", Charset, 0) != 0)
     goto cleanup;
 
   /* make sure that we can convert back and come out with the same
@@ -154,7 +154,7 @@ char *mutt_idna_intl_to_local(const char *user, const char *domain, int flags)
   {
     reversed_user = mutt_str_strdup(local_user);
 
-    if (mutt_ch_convert_string(&reversed_user, Charset, "utf-8", 0) == -1)
+    if (mutt_ch_convert_string(&reversed_user, Charset, "utf-8", 0) != 0)
     {
       mutt_debug(1, "Not reversible. Charset conv to utf-8 failed for user = '%s'.\n",
                  reversed_user);
@@ -169,7 +169,7 @@ char *mutt_idna_intl_to_local(const char *user, const char *domain, int flags)
 
     reversed_domain = mutt_str_strdup(local_domain);
 
-    if (mutt_ch_convert_string(&reversed_domain, Charset, "utf-8", 0) == -1)
+    if (mutt_ch_convert_string(&reversed_domain, Charset, "utf-8", 0) != 0)
     {
       mutt_debug(1, "Not reversible. Charset conv to utf-8 failed for domain = '%s'.\n",
                  reversed_domain);
@@ -236,10 +236,10 @@ char *mutt_idna_local_to_intl(const char *user, const char *domain)
   char *intl_domain = mutt_str_strdup(domain);
 
   /* we don't want charset-hook effects, so we set flags to 0 */
-  if (mutt_ch_convert_string(&intl_user, Charset, "utf-8", 0) == -1)
+  if (mutt_ch_convert_string(&intl_user, Charset, "utf-8", 0) != 0)
     goto cleanup;
 
-  if (mutt_ch_convert_string(&intl_domain, Charset, "utf-8", 0) == -1)
+  if (mutt_ch_convert_string(&intl_domain, Charset, "utf-8", 0) != 0)
     goto cleanup;
 
 #ifdef HAVE_LIBIDN

diff --git a/mutt/rfc2047.c b/mutt/rfc2047.c
index 47b44b5ef6c63bc830a08224b2ccae7f6ab51139..e9ef6d89f347a1ba5ede428e2d6be0805f044cd3 100644 (file)
--- a/mutt/rfc2047.c
+++ b/mutt/rfc2047.c
@@ -439,7 +439,7 @@ static int rfc2047_encode(const char *d, size_t dlen, int col, const char *fromc
 
   /* Try to convert to UTF-8. */
   char *u = mutt_str_substr_dup(d, d + dlen);
-  if (mutt_ch_convert_string(&u, fromcode, icode, 0))
+  if (mutt_ch_convert_string(&u, fromcode, icode, 0) != 0)
   {
     rc = 1;
     icode = 0;