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 <[email protected]>
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);
>From c7d9b146a201074b9559e9d5020333d680a1a5ec Mon Sep 17 00:00:00 2001
From: Bruno Haible <[email protected]>
Date: Mon, 9 May 2016 09:29:35 +0200
Subject: [PATCH] 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.
---
ChangeLog | 8 ++++++++
lib/gettext.h | 8 ++++++--
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index fe747e5..a8ba38e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2016-05-09 Bruno Haible <[email protected]>
+
+ 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.
+
2016-05-08 Paul Eggert <[email protected]>
git-version-gen: avoid undefined shift
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);
--
2.6.4
