bug-gnulib
[Top][All Lists]
Advanced

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

Re: [PATCH v2] gettext-h: avoid undefined behavior


From: Bruno Haible
Subject: Re: [PATCH v2] gettext-h: avoid undefined behavior
Date: Mon, 09 May 2016 09:33:05 +0200
User-agent: KMail/4.8.5 (Linux/3.8.0-44-generic; KDE/4.8.5; x86_64; ; )

Hi Daiki,

While your patch is correct, I don't like that much code that
  - does free(NULL) in the main execution path of the code,
  - does computations such as (n == 1 ? msgid : msgid_plural)
    before they turn out to be needed.
  - assigns a variable in several places in a single execution
    path. (I prefer variables that are assigned exactly once.
    That makes the code clearer.)

How about this patch instead? (Complete git change in attachment.)


2016-05-09  Bruno Haible  <address@hidden>

        Fix undefined behaviour in gettext.h.
        * lib/gettext.h (dcpgettext_expr, dcnpgettext_expr): Avoid accessing a
        pointer's value after the storage it points to has been freed.
        Reported by Michael Pyne in https://savannah.gnu.org/bugs/?47847.
        Spotted by Coverity.

diff --git a/lib/gettext.h b/lib/gettext.h
index 3acc6a6..d26b8b4 100644
--- a/lib/gettext.h
+++ b/lib/gettext.h
@@ -225,15 +225,17 @@ dcpgettext_expr (const char *domain,
   if (msg_ctxt_id != NULL)
 #endif
     {
+      int found_translation;
       memcpy (msg_ctxt_id, msgctxt, msgctxt_len - 1);
       msg_ctxt_id[msgctxt_len - 1] = '\004';
       memcpy (msg_ctxt_id + msgctxt_len, msgid, msgid_len);
       translation = dcgettext (domain, msg_ctxt_id, category);
+      found_translation = (translation != msg_ctxt_id);
 #if !_LIBGETTEXT_HAVE_VARIABLE_SIZE_ARRAYS
       if (msg_ctxt_id != buf)
         free (msg_ctxt_id);
 #endif
-      if (translation != msg_ctxt_id)
+      if (found_translation)
         return translation;
     }
   return msgid;
@@ -271,15 +273,17 @@ dcnpgettext_expr (const char *domain,
   if (msg_ctxt_id != NULL)
 #endif
     {
+      int found_translation;
       memcpy (msg_ctxt_id, msgctxt, msgctxt_len - 1);
       msg_ctxt_id[msgctxt_len - 1] = '\004';
       memcpy (msg_ctxt_id + msgctxt_len, msgid, msgid_len);
       translation = dcngettext (domain, msg_ctxt_id, msgid_plural, n, 
category);
+      found_translation = !(translation == msg_ctxt_id || translation == 
msgid_plural);
 #if !_LIBGETTEXT_HAVE_VARIABLE_SIZE_ARRAYS
       if (msg_ctxt_id != buf)
         free (msg_ctxt_id);
 #endif
-      if (!(translation == msg_ctxt_id || translation == msgid_plural))
+      if (found_translation)
         return translation;
     }
   return (n == 1 ? msgid : msgid_plural);

Attachment: 0001-Fix-undefined-behaviour-in-gettext.h.patch
Description: Text Data


reply via email to

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