Jesper Fehrlund <jes...@prisjakt.nu> writes:
> The first time a message is encountered, I can see that this would be
> the case.
> But, if the message already exists, then this code is executed:
>
> mp = message_list_search (mlp, msgctxt, msgid);
> if (mp != NULL)
> {
> if (msgctxt != NULL)
> free (msgctxt);
> free (msgid);
My bad. You are right. I've ended up with the attached patch.
Thanks for the good suggestions.
>From 84044b5fe692b6f32d73ad101327dd219887936b Mon Sep 17 00:00:00 2001
From: Daiki Ueno <u...@gnu.org>
Date: Tue, 28 Oct 2014 16:38:40 +0900
Subject: [PATCH] xgettext: Allow plural extraction from a single argument
function
The commit 8137d2b4 was a wrong fix since both singular/plural
msgids may point to the same address for Qt4 plural forms. This
reverts the commit and fix the original double-free problem in the
right way. Thanks to Jesper Fehrlund for suggestions.
* gettext-tools/src/xgettext.c (arglist_parser_remember_literal):
Don't ignore plural argument even if ARGNUM1 equals to ARGNUM2.
(arglist_parser_done): Make a copy of best_cp->msgid_plural when
passing it to remember_a_message_plural, if it equals to
best_cp->msgid. Also move code conversion logic earlier taking into
account of the ownership transfer of best_cp->msgid.
* gettext-tools/src/xgettext-12: Test a single argument function.
---
gettext-tools/src/ChangeLog | 14 +++++++++
gettext-tools/src/xgettext.c | 66 +++++++++++++++++++++++------------------
gettext-tools/tests/ChangeLog | 5 ++++
gettext-tools/tests/xgettext-12 | 15 +++++++++-
4 files changed, 70 insertions(+), 30 deletions(-)
diff --git a/gettext-tools/src/ChangeLog b/gettext-tools/src/ChangeLog
index e5ff794..d392ba7 100644
--- a/gettext-tools/src/ChangeLog
+++ b/gettext-tools/src/ChangeLog
@@ -1,5 +1,19 @@
2014-10-28 Daiki Ueno <u...@gnu.org>
+ xgettext: Allow plural extraction from a single argument function
+ The commit 8137d2b4 was a wrong fix since both singular/plural msgids
+ may point to the same address for Qt4 plural forms. This reverts
+ the commit and fix the original double-free problem in the right
+ way. Thanks to Jesper Fehrlund for suggestions.
+ * xgettext.c (arglist_parser_remember_literal): Don't ignore
+ plural argument even if ARGNUM1 equals to ARGNUM2.
+ (arglist_parser_done): Make a copy of best_cp->msgid_plural when
+ passing it to remember_a_message_plural, if it equals to
+ best_cp->msgid. Also move code conversion logic earlier taking
+ into account of the ownership transfer of best_cp->msgid.
+
+2014-10-28 Daiki Ueno <u...@gnu.org>
+
xgettext: Fix double-free in singular/plural argument extraction
After commit 6aa7b7ed in 2009, xgettext assumed that ARGNUM1 and
ARGNUM2 of -k are different. That could cause an double-free in
diff --git a/gettext-tools/src/xgettext.c b/gettext-tools/src/xgettext.c
index a8f3af6..7a4f71a 100644
--- a/gettext-tools/src/xgettext.c
+++ b/gettext-tools/src/xgettext.c
@@ -2791,7 +2791,7 @@ arglist_parser_remember_literal (struct arglist_parser *ap,
/* Mark msgid as done. */
cp->argnum1 = 0;
}
- else if (argnum == cp->argnum2)
+ if (argnum == cp->argnum2)
{
cp->msgid_plural = string;
cp->msgid_plural_escape = type;
@@ -3099,9 +3099,9 @@ arglist_parser_done (struct arglist_parser *ap, int argnum)
char *msgid = parser->parse (best_cp->msgid,
&best_cp->msgid_pos,
best_cp->msgid_escape);
- free (best_cp->msgid);
if (best_cp->msgid_plural == best_cp->msgid)
best_cp->msgid_plural = msgid;
+ free (best_cp->msgid);
best_cp->msgid = msgid;
}
else
@@ -3110,26 +3110,7 @@ arglist_parser_done (struct arglist_parser *ap, int argnum)
CONVERT_STRING (best_cp->msgid, lc_string);
}
- if (best_cp->msgid_comment != NULL)
- {
- refcounted_string_list_ty *msgid_comment =
- savable_comment_convert_encoding (best_cp->msgid_comment,
- &best_cp->msgid_pos);
- drop_reference (best_cp->msgid_comment);
- best_cp->msgid_comment = msgid_comment;
- }
-
- /* best_cp->msgctxt and best_cp->msgid are already in
- UTF-8. Prevent further conversion in remember_a_message. */
- encoding = xgettext_current_source_encoding;
- xgettext_current_source_encoding = po_charset_utf8;
- mp = remember_a_message (ap->mlp, best_cp->msgctxt, best_cp->msgid,
- msgid_context,
- &best_cp->msgid_pos,
- NULL, best_cp->msgid_comment);
- xgettext_current_source_encoding = encoding;
-
- if (mp != NULL && best_cp->msgid_plural != NULL)
+ if (best_cp->msgid_plural)
{
/* best_cp->msgid_plural may point to best_cp->msgid.
In that case, it is already interpreted and converted. */
@@ -3152,14 +3133,41 @@ arglist_parser_done (struct arglist_parser *ap, int argnum)
}
}
- encoding = xgettext_current_source_encoding;
- xgettext_current_source_encoding = po_charset_utf8;
- remember_a_message_plural (mp, best_cp->msgid_plural,
- msgid_plural_context,
- &best_cp->msgid_plural_pos,
- NULL);
- xgettext_current_source_encoding = encoding;
+ /* If best_cp->msgid_plural equals to best_cp->msgid,
+ the ownership will be transferred to
+ remember_a_message before it is passed to
+ remember_a_message_plural.
+
+ Make a copy of the string in that case. */
+ if (best_cp->msgid_plural == best_cp->msgid)
+ best_cp->msgid_plural = xstrdup (best_cp->msgid);
+ }
+
+ if (best_cp->msgid_comment != NULL)
+ {
+ refcounted_string_list_ty *msgid_comment =
+ savable_comment_convert_encoding (best_cp->msgid_comment,
+ &best_cp->msgid_pos);
+ drop_reference (best_cp->msgid_comment);
+ best_cp->msgid_comment = msgid_comment;
}
+
+ /* best_cp->msgctxt, best_cp->msgid, and best_cp->msgid_plural
+ are already in UTF-8. Prevent further conversion in
+ remember_a_message. */
+ encoding = xgettext_current_source_encoding;
+ xgettext_current_source_encoding = po_charset_utf8;
+ mp = remember_a_message (ap->mlp, best_cp->msgctxt, best_cp->msgid,
+ msgid_context,
+ &best_cp->msgid_pos,
+ NULL, best_cp->msgid_comment);
+ if (mp != NULL && best_cp->msgid_plural != NULL)
+ remember_a_message_plural (mp,
+ best_cp->msgid_plural,
+ msgid_plural_context,
+ &best_cp->msgid_plural_pos,
+ NULL);
+ xgettext_current_source_encoding = encoding;
}
if (best_cp->xcomments.nitems > 0)
diff --git a/gettext-tools/tests/ChangeLog b/gettext-tools/tests/ChangeLog
index 8fb2051..d6d6788 100644
--- a/gettext-tools/tests/ChangeLog
+++ b/gettext-tools/tests/ChangeLog
@@ -1,5 +1,10 @@
2014-10-28 Daiki Ueno <u...@gnu.org>
+ xgettext: Allow plural extraction from a single argument function
+ * xgettext-12: Test a single argument function.
+
+2014-10-28 Daiki Ueno <u...@gnu.org>
+
tests: Add test for same ARGNUM1/ARGNUM2 given to xgettext -k
* xgettext-12: New file.
* Makefile.am (TESTS): Add new test.
diff --git a/gettext-tools/tests/xgettext-12 b/gettext-tools/tests/xgettext-12
index 0438ca7..fe76f1a 100755
--- a/gettext-tools/tests/xgettext-12
+++ b/gettext-tools/tests/xgettext-12
@@ -7,10 +7,15 @@
cat <<\EOF > xg-test12.c
ngettext ("abc", "abc", 1);
ngettext ("abc", "abc", 1);
+
+/* Hypothetical case: plural extraction from a function
+ with a single argument. */
+xngettext ("def", 1);
+xngettext ("def", 1);
EOF
: ${XGETTEXT=xgettext}
-${XGETTEXT} --omit-header --add-comments -kngettext:1,1 -d xg-test12.tmp xg-test12.c || exit 1
+${XGETTEXT} --omit-header --add-comments -kngettext:1,1 -kxngettext:1,1 -d xg-test12.tmp xg-test12.c || exit 1
LC_ALL=C tr -d '\r' < xg-test12.tmp.po > xg-test12.po || exit 1
cat <<\EOF > xg-test12.ok
@@ -19,6 +24,14 @@ msgid "abc"
msgid_plural "abc"
msgstr[0] ""
msgstr[1] ""
+
+#. Hypothetical case: plural extraction from a function
+#. with a single argument.
+#: xg-test12.c:6 xg-test12.c:7
+msgid "def"
+msgid_plural "def"
+msgstr[0] ""
+msgstr[1] ""
EOF
: ${DIFF=diff}
--
2.1.1
Regards,
--
Daiki Ueno
