GNU bug report logs - #26462
Fix crashes in programs using system ICU

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 26462 in the body.
You can then email your comments to 26462 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Clément Lassieur <clement <at> lassieur.org>
To: guix-patches <at> gnu.org
Subject: Fix crashes in programs using system ICU
Date: Wed, 12 Apr 2017 12:48:20 +0200

The first patch fixes system ICU.  This allows Icecat to use system ICU
without crashing, and it also fixes crashes in 0ad, which I'm working
on.

Message #8 received at 26462 <at> debbugs.gnu.org (full text, mbox):

From: Clément Lassieur <clement <at> lassieur.org>
To: 26462 <at> debbugs.gnu.org
Subject: [PATCH 1/2] gnu: icu4c: Fix crashes in programs using system ICU.
Date: Wed, 12 Apr 2017 12:53:24 +0200

* gnu/packages/patches/icu4c-reset-keyword-list-iterator.patch: New file.
* gnu/local.mk (dist_patch_DATA): Add it.
* gnu/packages/icu4c.scm (icu4c)[source]: Use it.
---
 gnu/local.mk                                       |   1 +
 gnu/packages/icu4c.scm                             |   5 +-
 .../icu4c-reset-keyword-list-iterator.patch        | 128 +++++++++++++++++++++
 3 files changed, 133 insertions(+), 1 deletion(-)
 create mode 100644 gnu/packages/patches/icu4c-reset-keyword-list-iterator.patch

diff --git a/gnu/local.mk b/gnu/local.mk
index 98dffff64..a6f3448a2 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -658,6 +658,7 @@ dist_patch_DATA =						\
   %D%/packages/patches/icu4c-CVE-2014-6585.patch		\
   %D%/packages/patches/icu4c-CVE-2015-1270.patch		\
   %D%/packages/patches/icu4c-CVE-2015-4760.patch		\
+  %D%/packages/patches/icu4c-reset-keyword-list-iterator.patch	\
   %D%/packages/patches/id3lib-CVE-2007-4460.patch			\
   %D%/packages/patches/ilmbase-fix-tests.patch			\
   %D%/packages/patches/isl-0.11.1-aarch64-support.patch	\
diff --git a/gnu/packages/icu4c.scm b/gnu/packages/icu4c.scm
index d842f03b4..3283811a0 100644
--- a/gnu/packages/icu4c.scm
+++ b/gnu/packages/icu4c.scm
@@ -2,6 +2,7 @@
 ;;; Copyright © 2013 Andreas Enge <andreas <at> enge.fr>
 ;;; Copyright © 2015, 2016 Mark H Weaver <mhw <at> netris.org>
 ;;; Copyright © 2016 Efraim Flashner <efraim <at> flashner.co.il>
+;;; Copyright © 2017 Clément Lassieur <clement <at> lassieur.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -39,7 +40,9 @@
                   (string-map (lambda (x) (if (char=? x #\.) #\_ x)) version)
                   "-src.tgz"))
             (sha256
-             (base32 "036shcb3f8bm1lynhlsb4kpjm9s9c2vdiir01vg216rs2l8482ib"))))
+             (base32 "036shcb3f8bm1lynhlsb4kpjm9s9c2vdiir01vg216rs2l8482ib"))
+            (patches
+             (search-patches "icu4c-reset-keyword-list-iterator.patch"))))
    (build-system gnu-build-system)
    (inputs
     `(("perl" ,perl)))
diff --git a/gnu/packages/patches/icu4c-reset-keyword-list-iterator.patch b/gnu/packages/patches/icu4c-reset-keyword-list-iterator.patch
new file mode 100644
index 000000000..ddedc56de
--- /dev/null
+++ b/gnu/packages/patches/icu4c-reset-keyword-list-iterator.patch
@@ -0,0 +1,128 @@
+Copied from upstream: http://bugs.icu-project.org/trac/changeset/39484/.
+
+Paths and line endings have been adapted.
+
+Index: icu/source/common/ulist.c
+===================================================================
+--- icu/source/common/ulist.c	(revision 39483)
++++ icu/source/common/ulist.c	(revision 39484)
+@@ -30,5 +30,4 @@
+     
+     int32_t size;
+-    int32_t currentIndex;
+ };
+ 
+@@ -52,5 +51,4 @@
+     newList->tail = NULL;
+     newList->size = 0;
+-    newList->currentIndex = -1;
+     
+     return newList;
+@@ -81,6 +79,7 @@
+         p->next->previous = p->previous;
+     }
+-    list->curr = NULL;
+-    list->currentIndex = 0;
++    if (p == list->curr) {
++        list->curr = p->next;
++    }
+     --list->size;
+     if (p->forceDelete) {
+@@ -151,5 +150,4 @@
+         list->head->previous = newItem;
+         list->head = newItem;
+-        list->currentIndex++;
+     }
+     
+@@ -194,5 +192,4 @@
+     curr = list->curr;
+     list->curr = curr->next;
+-    list->currentIndex++;
+     
+     return curr->data;
+@@ -210,5 +207,4 @@
+     if (list != NULL) {
+         list->curr = list->head;
+-        list->currentIndex = 0;
+     }
+ }
+@@ -273,3 +269,2 @@
+     return (UList *)(en->context);
+ }
+-
+Index: icu/source/i18n/ucol_res.cpp
+===================================================================
+--- icu/source/i18n/ucol_res.cpp	(revision 39483)
++++ icu/source/i18n/ucol_res.cpp	(revision 39484)
+@@ -681,4 +681,5 @@
+     }
+     memcpy(en, &defaultKeywordValues, sizeof(UEnumeration));
++    ulist_resetList(sink.values);  // Initialize the iterator.
+     en->context = sink.values;
+     sink.values = NULL;  // Avoid deletion in the sink destructor.
+Index: icu/source/test/intltest/apicoll.cpp
+===================================================================
+--- icu/source/test/intltest/apicoll.cpp	(revision 39483)
++++ icu/source/test/intltest/apicoll.cpp	(revision 39484)
+@@ -82,14 +82,7 @@
+     col = Collator::createInstance(Locale::getEnglish(), success);
+     if (U_FAILURE(success)){
+-        errcheckln(success, "Default Collator creation failed. - %s", u_errorName(success));
+-        return;
+-    }
+-
+-    StringEnumeration* kwEnum = col->getKeywordValuesForLocale("", Locale::getEnglish(),true,success);
+-    if (U_FAILURE(success)){
+-        errcheckln(success, "Get Keyword Values for Locale failed. - %s", u_errorName(success));
+-        return;
+-    }
+-    delete kwEnum;
++        errcheckln(success, "English Collator creation failed. - %s", u_errorName(success));
++        return;
++    }
+ 
+     col->getVersion(versionArray);
+@@ -230,4 +223,27 @@
+     delete aFrCol;
+     delete junk;
++}
++
++void CollationAPITest::TestKeywordValues() {
++    IcuTestErrorCode errorCode(*this, "TestKeywordValues");
++    LocalPointer<Collator> col(Collator::createInstance(Locale::getEnglish(), errorCode));
++    if (errorCode.logIfFailureAndReset("English Collator creation failed")) {
++        return;
++    }
++
++    LocalPointer<StringEnumeration> kwEnum(
++        col->getKeywordValuesForLocale("collation", Locale::getEnglish(), TRUE, errorCode));
++    if (errorCode.logIfFailureAndReset("Get Keyword Values for English Collator failed")) {
++        return;
++    }
++    assertTrue("expect at least one collation tailoring for English", kwEnum->count(errorCode) > 0);
++    const char *kw;
++    UBool hasStandard = FALSE;
++    while ((kw = kwEnum->next(NULL, errorCode)) != NULL) {
++        if (strcmp(kw, "standard") == 0) {
++            hasStandard = TRUE;
++        }
++    }
++    assertTrue("expect at least the 'standard' collation tailoring for English", hasStandard);
+ }
+ 
+@@ -2467,4 +2483,5 @@
+     TESTCASE_AUTO_BEGIN;
+     TESTCASE_AUTO(TestProperty);
++    TESTCASE_AUTO(TestKeywordValues);
+     TESTCASE_AUTO(TestOperators);
+     TESTCASE_AUTO(TestDuplicate);
+Index: icu/source/test/intltest/apicoll.h
+===================================================================
+--- icu/source/test/intltest/apicoll.h	(revision 39483)
++++ icu/source/test/intltest/apicoll.h	(revision 39484)
+@@ -36,4 +36,5 @@
+      */
+     void TestProperty(/* char* par */);
++    void TestKeywordValues();
+ 
+     /**
-- 
2.12.2

Message #11 received at 26462 <at> debbugs.gnu.org (full text, mbox):

From: Clément Lassieur <clement <at> lassieur.org>
To: 26462 <at> debbugs.gnu.org
Subject: [PATCH 2/2] gnu: icecat: Use system ICU.
Date: Wed, 12 Apr 2017 12:53:25 +0200

* gnu/packages/gnuzilla.scm (icecat)[arguments] <#:configure-flags>: Remove
comment for "--with-system-icu".
---
 gnu/packages/gnuzilla.scm | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/gnu/packages/gnuzilla.scm b/gnu/packages/gnuzilla.scm
index 4a2e784ee..a6c1a07cf 100644
--- a/gnu/packages/gnuzilla.scm
+++ b/gnu/packages/gnuzilla.scm
@@ -509,11 +509,7 @@ standards.")
                            "--with-system-jpeg"        ; must be libjpeg-turbo
                            "--with-system-libevent"
                            "--with-system-libvpx"
-
-                           ;; FIXME: It is preferable to build with system
-                           ;; libraries, but this fixes crashes.
-                           ;; "--with-system-icu"
-
+                           "--with-system-icu"
                            "--with-system-nspr"
                            "--with-system-nss"
                            "--enable-system-pixman"
-- 
2.12.2

Message #14 received at 26462 <at> debbugs.gnu.org (full text, mbox):

From: Clément Lassieur <clement <at> lassieur.org>
To: 26462 <at> debbugs.gnu.org
Subject: Re: bug#26462: Fix crashes in programs using system ICU
Date: Wed, 12 Apr 2017 12:55:21 +0200

Clément Lassieur <clement <at> lassieur.org> writes:

> The first patch fixes system ICU.  This allows Icecat to use system ICU
> without crashing, and it also fixes crashes in 0ad, which I'm working
> on.

They should probably be applied to core-updates.

Message #17 received at 26462 <at> debbugs.gnu.org (full text, mbox):

From: ng0 <contact.ng0 <at> cryptolab.net>
To: Clément Lassieur <clement <at> lassieur.org>
Cc: 26462 <at> debbugs.gnu.org
Subject: Re: bug#26462: Fix crashes in programs using system ICU
Date: Wed, 12 Apr 2017 11:22:15 +0000

Clément Lassieur transcribed 0.1K bytes:
> The first patch fixes system ICU.  This allows Icecat to use system ICU

Cool :)

> without crashing, and it also fixes crashes in 0ad, which I'm working
> on.
> 
> 
> 
Woooo. You picked up my 0ad work (or started from scratch)? I was stuck on mozjs38 for an eternity
and gave up.
Do you need further help or something? I unpublished the repo this was
in, so the last progress isn't public.
-- 
PGP and more: https://people.pragmatique.xyz/ng0/

Message #20 received at 26462 <at> debbugs.gnu.org (full text, mbox):

From: Clément Lassieur <clement <at> lassieur.org>
To: ng0 <contact.ng0 <at> cryptolab.net>
Cc: 26462 <at> debbugs.gnu.org
Subject: Re: bug#26462: Fix crashes in programs using system ICU
Date: Wed, 12 Apr 2017 13:31:40 +0200

ng0 <contact.ng0 <at> cryptolab.net> writes:

> Clément Lassieur transcribed 0.1K bytes:
>> The first patch fixes system ICU.  This allows Icecat to use system ICU
>
> Cool :)
>
>> without crashing, and it also fixes crashes in 0ad, which I'm working
>> on.
>> 
>> 
>> 
> Woooo. You picked up my 0ad work (or started from scratch)? I was stuck on mozjs38 for an eternity
> and gave up.
> Do you need further help or something? I unpublished the repo this was
> in, so the last progress isn't public.

:-) I picked up your work, of course.  The mozjs issue is fixed (a patch
that was applied to the bundled mozjs had to be applied to the system
mozjs we use), and with this ICU patch, I can play!  I just played for
5min without a problem.  I just need to cleanup things, to package NVTT
(did you do it?), and then I can send the patch.

Message #23 received at 26462 <at> debbugs.gnu.org (full text, mbox):

From: Clément Lassieur <clement <at> lassieur.org>
To: ng0 <contact.ng0 <at> cryptolab.net>
Cc: 26462 <at> debbugs.gnu.org
Subject: Re: bug#26462: Fix crashes in programs using system ICU
Date: Wed, 12 Apr 2017 13:42:57 +0200

Clément Lassieur <clement <at> lassieur.org> writes:

> ng0 <contact.ng0 <at> cryptolab.net> writes:
>
>> Clément Lassieur transcribed 0.1K bytes:
>>> The first patch fixes system ICU.  This allows Icecat to use system ICU
>>
>> Cool :)
>>
>>> without crashing, and it also fixes crashes in 0ad, which I'm working
>>> on.
>>> 
>>> 
>>> 
>> Woooo. You picked up my 0ad work (or started from scratch)? I was stuck on mozjs38 for an eternity
>> and gave up.
>> Do you need further help or something? I unpublished the repo this was
>> in, so the last progress isn't public.

I'd like to have a look at your unpublished work, if this is possible.
:-)

> :-) I picked up your work, of course.  The mozjs issue is fixed (a patch
> that was applied to the bundled mozjs had to be applied to the system
> mozjs we use), and with this ICU patch, I can play!  I just played for
> 5min without a problem.  I just need to cleanup things, to package NVTT
> (did you do it?), and then I can send the patch.

Message #26 received at 26462 <at> debbugs.gnu.org (full text, mbox):

From: ng0 <contact.ng0 <at> cryptolab.net>
To: Clément Lassieur <clement <at> lassieur.org>
Cc: 26462 <at> debbugs.gnu.org
Subject: Re: bug#26462: Fix crashes in programs using system ICU
Date: Wed, 12 Apr 2017 11:47:56 +0000

Clément Lassieur transcribed 0.8K bytes:
> ng0 <contact.ng0 <at> cryptolab.net> writes:
> 
> > Clément Lassieur transcribed 0.1K bytes:
> >> The first patch fixes system ICU.  This allows Icecat to use system ICU
> >
> > Cool :)
> >
> >> without crashing, and it also fixes crashes in 0ad, which I'm working
> >> on.
> >> 
> >> 
> >> 
> > Woooo. You picked up my 0ad work (or started from scratch)? I was stuck on mozjs38 for an eternity
> > and gave up.
> > Do you need further help or something? I unpublished the repo this was
> > in, so the last progress isn't public.
> 
> :-) I picked up your work, of course.  The mozjs issue is fixed (a patch
> that was applied to the bundled mozjs had to be applied to the system
> mozjs we use), and with this ICU patch, I can play!  I just played for
> 5min without a problem.  I just need to cleanup things, to package NVTT
> (did you do it?), and then I can send the patch.

Wow :) That is so exciting... really. And you only get to add the
package on top of every package list "0" and "a" once :D
I don't want to sound pushy or anything, I don't even know how much I
messed up or not, but it would be cool to mention me as co-authored
depending on the code :)
If you don't do it, it's fine aswell because I'm so happy you picked it up!

No, I left NVTT as "to do". But when I had an github account, I made
Nvidia publish a new release for NVTT. So it's just a matter of
packaging the NVTT dependencies. This should not block the 0ad package
commit in my opinion. As no release happened for a long time - until I requested them to
do so - every project which makes use of NVTT just bundled a working state of NVTT.


-- 
PGP and more: https://people.pragmatique.xyz/ng0/

Message #29 received at 26462 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Clément Lassieur <clement <at> lassieur.org>
Cc: 26462 <at> debbugs.gnu.org
Subject: Re: bug#26462: [PATCH 1/2] gnu: icu4c: Fix crashes in programs using
 system ICU.
Date: Wed, 12 Apr 2017 13:51:11 +0200

Clément Lassieur <clement <at> lassieur.org> skribis:

> * gnu/packages/patches/icu4c-reset-keyword-list-iterator.patch: New file.
> * gnu/local.mk (dist_patch_DATA): Add it.
> * gnu/packages/icu4c.scm (icu4c)[source]: Use it.

[...]

> +++ b/gnu/packages/patches/icu4c-reset-keyword-list-iterator.patch
> @@ -0,0 +1,128 @@
> +Copied from upstream: http://bugs.icu-project.org/trac/changeset/39484/.
> +
> +Paths and line endings have been adapted.

Woow, good catch!

1,700 packages depend on icu4c so the update would have to go to
‘core-updates’, but that won’t be merged until several weeks.

For ‘master’, in the meantime, I see two solutions:

  1. If the bug only affects specific applications, we could add an
     icu4c variant and use that only where needed (icecat, 0ad, etc.).

  2. If the bug manifests itself pervasively we could graft it in
     ‘master’ (I guess it’s ABI-compatible, right?).

Thoughts?

Thank you for addressing it!

Ludo’.

Message #32 received at 26462 <at> debbugs.gnu.org (full text, mbox):

From: ng0 <contact.ng0 <at> cryptolab.net>
To: Clément Lassieur <clement <at> lassieur.org>
Cc: 26462 <at> debbugs.gnu.org
Subject: Re: bug#26462: Fix crashes in programs using system ICU
Date: Wed, 12 Apr 2017 11:53:54 +0000

Clément Lassieur transcribed 1.0K bytes:
> Clément Lassieur <clement <at> lassieur.org> writes:
> 
> > ng0 <contact.ng0 <at> cryptolab.net> writes:
> >
> >> Clément Lassieur transcribed 0.1K bytes:
> >>> The first patch fixes system ICU.  This allows Icecat to use system ICU
> >>
> >> Cool :)
> >>
> >>> without crashing, and it also fixes crashes in 0ad, which I'm working
> >>> on.
> >>> 
> >>> 
> >>> 
> >> Woooo. You picked up my 0ad work (or started from scratch)? I was stuck on mozjs38 for an eternity
> >> and gave up.
> >> Do you need further help or something? I unpublished the repo this was
> >> in, so the last progress isn't public.
> 
> I'd like to have a look at your unpublished work, if this is possible.
> :-)

I was wrong. It is published, I just moved the branch into my personal
main repo.
You can find it in the 'package/games/0ad' branch at
https://gitweb.pragmatique.xyz/pragmatique/guix.git/ (which will be
moved within the next few days/weeks to a pagure instance). Naming was
unfortunate, it is my personal "pick this up for inspiration or work" repo :)

> > :-) I picked up your work, of course.  The mozjs issue is fixed (a patch
> > that was applied to the bundled mozjs had to be applied to the system
> > mozjs we use), and with this ICU patch, I can play!  I just played for
> > 5min without a problem.  I just need to cleanup things, to package NVTT
> > (did you do it?), and then I can send the patch.
> 

-- 
PGP and more: https://people.pragmatique.xyz/ng0/

Message #35 received at 26462 <at> debbugs.gnu.org (full text, mbox):

From: Clément Lassieur <clement <at> lassieur.org>
To: ng0 <contact.ng0 <at> cryptolab.net>
Cc: 26462 <at> debbugs.gnu.org
Subject: Re: bug#26462: Fix crashes in programs using system ICU
Date: Fri, 14 Apr 2017 00:53:13 +0200

ng0 <contact.ng0 <at> cryptolab.net> writes:

> Clément Lassieur transcribed 1.0K bytes:
>
>> I'd like to have a look at your unpublished work, if this is possible.
>> :-)
>>
> I was wrong. It is published, I just moved the branch into my personal
> main repo.
> You can find it in the 'package/games/0ad' branch at
> https://gitweb.pragmatique.xyz/pragmatique/guix.git/ (which will be
> moved within the next few days/weeks to a pagure instance). Naming was
> unfortunate, it is my personal "pick this up for inspiration or work" repo :)

I cloned it.  Thanks!

Message #38 received at 26462 <at> debbugs.gnu.org (full text, mbox):

From: Clément Lassieur <clement <at> lassieur.org>
To: ng0 <contact.ng0 <at> cryptolab.net>
Cc: 26462 <at> debbugs.gnu.org
Subject: Re: bug#26462: Fix crashes in programs using system ICU
Date: Fri, 14 Apr 2017 01:01:08 +0200

ng0 <contact.ng0 <at> cryptolab.net> writes:
> Clément Lassieur transcribed 0.8K bytes:
>> ng0 <contact.ng0 <at> cryptolab.net> writes:
>> 
>> > Clément Lassieur transcribed 0.1K bytes:
>> >> The first patch fixes system ICU.  This allows Icecat to use system ICU
>> >
>> > Cool :)
>> >
>> >> without crashing, and it also fixes crashes in 0ad, which I'm working
>> >> on.
>> >> 
>> >> 
>> >> 
>> > Woooo. You picked up my 0ad work (or started from scratch)? I was stuck on mozjs38 for an eternity
>> > and gave up.
>> > Do you need further help or something? I unpublished the repo this was
>> > in, so the last progress isn't public.
>> 
>> :-) I picked up your work, of course.  The mozjs issue is fixed (a patch
>> that was applied to the bundled mozjs had to be applied to the system
>> mozjs we use), and with this ICU patch, I can play!  I just played for
>> 5min without a problem.  I just need to cleanup things, to package NVTT
>> (did you do it?), and then I can send the patch.
>
> Wow :) That is so exciting... really. And you only get to add the
> package on top of every package list "0" and "a" once :D

Cool! ;)

> I don't want to sound pushy or anything, I don't even know how much I
> messed up or not, but it would be cool to mention me as co-authored
> depending on the code :)
> If you don't do it, it's fine aswell because I'm so happy you picked it up!

Of course I'll do it!  You did most of the work :)

> No, I left NVTT as "to do". But when I had an github account, I made
> Nvidia publish a new release for NVTT. So it's just a matter of
> packaging the NVTT dependencies. This should not block the 0ad package
> commit in my opinion. As no release happened for a long time - until I requested them to
> do so - every project which makes use of NVTT just bundled a working state of NVTT.

Ok, so I'll let it as is if you think it's fine.

Message #41 received at 26462 <at> debbugs.gnu.org (full text, mbox):

From: Clément Lassieur <clement <at> lassieur.org>
To: 26462 <at> debbugs.gnu.org
Subject: [PATCH] gnu: icu4c: Fix crashes in programs using system ICU.
Date: Fri, 14 Apr 2017 16:51:17 +0200

* gnu/packages/patches/icu4c-reset-keyword-list-iterator.patch: New file.
* gnu/local.mk (dist_patch_DATA): Add it.
* gnu/packages/icu4c.scm (icu4c)[replacement]: New field.
(icu4c/fixed): New variable.
---
 gnu/local.mk                                       |   1 +
 gnu/packages/icu4c.scm                             |  11 ++
 .../icu4c-reset-keyword-list-iterator.patch        | 128 +++++++++++++++++++++
 3 files changed, 140 insertions(+)
 create mode 100644 gnu/packages/patches/icu4c-reset-keyword-list-iterator.patch

diff --git a/gnu/local.mk b/gnu/local.mk
index 82e94171b..a006000a2 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -657,6 +657,7 @@ dist_patch_DATA =						\
   %D%/packages/patches/icu4c-CVE-2014-6585.patch		\
   %D%/packages/patches/icu4c-CVE-2015-1270.patch		\
   %D%/packages/patches/icu4c-CVE-2015-4760.patch		\
+  %D%/packages/patches/icu4c-reset-keyword-list-iterator.patch	\
   %D%/packages/patches/id3lib-CVE-2007-4460.patch			\
   %D%/packages/patches/ilmbase-fix-tests.patch			\
   %D%/packages/patches/isl-0.11.1-aarch64-support.patch	\
diff --git a/gnu/packages/icu4c.scm b/gnu/packages/icu4c.scm
index d842f03b4..2b5144100 100644
--- a/gnu/packages/icu4c.scm
+++ b/gnu/packages/icu4c.scm
@@ -2,6 +2,7 @@
 ;;; Copyright © 2013 Andreas Enge <andreas <at> enge.fr>
 ;;; Copyright © 2015, 2016 Mark H Weaver <mhw <at> netris.org>
 ;;; Copyright © 2016 Efraim Flashner <efraim <at> flashner.co.il>
+;;; Copyright © 2017 Clément Lassieur <clement <at> lassieur.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -30,6 +31,7 @@
   (package
    (name "icu4c")
    (version "58.2")
+   (replacement icu4c/fixed)
    (source (origin
             (method url-fetch)
             (uri (string-append
@@ -63,3 +65,12 @@ globalisation support for software applications.  This package contains the
 C/C++ part.")
    (license x11)
    (home-page "http://site.icu-project.org/")))
+
+(define icu4c/fixed
+  (package
+    (inherit icu4c)
+    (replacement #f)
+    (source (origin
+              (inherit (package-source icu4c))
+              (patches
+               (search-patches "icu4c-reset-keyword-list-iterator.patch"))))))
diff --git a/gnu/packages/patches/icu4c-reset-keyword-list-iterator.patch b/gnu/packages/patches/icu4c-reset-keyword-list-iterator.patch
new file mode 100644
index 000000000..ddedc56de
--- /dev/null
+++ b/gnu/packages/patches/icu4c-reset-keyword-list-iterator.patch
@@ -0,0 +1,128 @@
+Copied from upstream: http://bugs.icu-project.org/trac/changeset/39484/.
+
+Paths and line endings have been adapted.
+
+Index: icu/source/common/ulist.c
+===================================================================
+--- icu/source/common/ulist.c	(revision 39483)
++++ icu/source/common/ulist.c	(revision 39484)
+@@ -30,5 +30,4 @@
+     
+     int32_t size;
+-    int32_t currentIndex;
+ };
+ 
+@@ -52,5 +51,4 @@
+     newList->tail = NULL;
+     newList->size = 0;
+-    newList->currentIndex = -1;
+     
+     return newList;
+@@ -81,6 +79,7 @@
+         p->next->previous = p->previous;
+     }
+-    list->curr = NULL;
+-    list->currentIndex = 0;
++    if (p == list->curr) {
++        list->curr = p->next;
++    }
+     --list->size;
+     if (p->forceDelete) {
+@@ -151,5 +150,4 @@
+         list->head->previous = newItem;
+         list->head = newItem;
+-        list->currentIndex++;
+     }
+     
+@@ -194,5 +192,4 @@
+     curr = list->curr;
+     list->curr = curr->next;
+-    list->currentIndex++;
+     
+     return curr->data;
+@@ -210,5 +207,4 @@
+     if (list != NULL) {
+         list->curr = list->head;
+-        list->currentIndex = 0;
+     }
+ }
+@@ -273,3 +269,2 @@
+     return (UList *)(en->context);
+ }
+-
+Index: icu/source/i18n/ucol_res.cpp
+===================================================================
+--- icu/source/i18n/ucol_res.cpp	(revision 39483)
++++ icu/source/i18n/ucol_res.cpp	(revision 39484)
+@@ -681,4 +681,5 @@
+     }
+     memcpy(en, &defaultKeywordValues, sizeof(UEnumeration));
++    ulist_resetList(sink.values);  // Initialize the iterator.
+     en->context = sink.values;
+     sink.values = NULL;  // Avoid deletion in the sink destructor.
+Index: icu/source/test/intltest/apicoll.cpp
+===================================================================
+--- icu/source/test/intltest/apicoll.cpp	(revision 39483)
++++ icu/source/test/intltest/apicoll.cpp	(revision 39484)
+@@ -82,14 +82,7 @@
+     col = Collator::createInstance(Locale::getEnglish(), success);
+     if (U_FAILURE(success)){
+-        errcheckln(success, "Default Collator creation failed. - %s", u_errorName(success));
+-        return;
+-    }
+-
+-    StringEnumeration* kwEnum = col->getKeywordValuesForLocale("", Locale::getEnglish(),true,success);
+-    if (U_FAILURE(success)){
+-        errcheckln(success, "Get Keyword Values for Locale failed. - %s", u_errorName(success));
+-        return;
+-    }
+-    delete kwEnum;
++        errcheckln(success, "English Collator creation failed. - %s", u_errorName(success));
++        return;
++    }
+ 
+     col->getVersion(versionArray);
+@@ -230,4 +223,27 @@
+     delete aFrCol;
+     delete junk;
++}
++
++void CollationAPITest::TestKeywordValues() {
++    IcuTestErrorCode errorCode(*this, "TestKeywordValues");
++    LocalPointer<Collator> col(Collator::createInstance(Locale::getEnglish(), errorCode));
++    if (errorCode.logIfFailureAndReset("English Collator creation failed")) {
++        return;
++    }
++
++    LocalPointer<StringEnumeration> kwEnum(
++        col->getKeywordValuesForLocale("collation", Locale::getEnglish(), TRUE, errorCode));
++    if (errorCode.logIfFailureAndReset("Get Keyword Values for English Collator failed")) {
++        return;
++    }
++    assertTrue("expect at least one collation tailoring for English", kwEnum->count(errorCode) > 0);
++    const char *kw;
++    UBool hasStandard = FALSE;
++    while ((kw = kwEnum->next(NULL, errorCode)) != NULL) {
++        if (strcmp(kw, "standard") == 0) {
++            hasStandard = TRUE;
++        }
++    }
++    assertTrue("expect at least the 'standard' collation tailoring for English", hasStandard);
+ }
+ 
+@@ -2467,4 +2483,5 @@
+     TESTCASE_AUTO_BEGIN;
+     TESTCASE_AUTO(TestProperty);
++    TESTCASE_AUTO(TestKeywordValues);
+     TESTCASE_AUTO(TestOperators);
+     TESTCASE_AUTO(TestDuplicate);
+Index: icu/source/test/intltest/apicoll.h
+===================================================================
+--- icu/source/test/intltest/apicoll.h	(revision 39483)
++++ icu/source/test/intltest/apicoll.h	(revision 39484)
+@@ -36,4 +36,5 @@
+      */
+     void TestProperty(/* char* par */);
++    void TestKeywordValues();
+ 
+     /**
-- 
2.12.2

Message #44 received at 26462 <at> debbugs.gnu.org (full text, mbox):

From: Clément Lassieur <clement <at> lassieur.org>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 26462 <at> debbugs.gnu.org
Subject: Re: bug#26462: [PATCH 1/2] gnu: icu4c: Fix crashes in programs using
 system ICU.
Date: Fri, 14 Apr 2017 16:55:37 +0200

Ludovic Courtès <ludo <at> gnu.org> writes:

> Clément Lassieur <clement <at> lassieur.org> skribis:
>
>> * gnu/packages/patches/icu4c-reset-keyword-list-iterator.patch: New file.
>> * gnu/local.mk (dist_patch_DATA): Add it.
>> * gnu/packages/icu4c.scm (icu4c)[source]: Use it.
>
> [...]
>
>> +++ b/gnu/packages/patches/icu4c-reset-keyword-list-iterator.patch
>> @@ -0,0 +1,128 @@
>> +Copied from upstream: http://bugs.icu-project.org/trac/changeset/39484/.
>> +
>> +Paths and line endings have been adapted.
>
> Woow, good catch!
>
> 1,700 packages depend on icu4c so the update would have to go to
> ‘core-updates’, but that won’t be merged until several weeks.
>
> For ‘master’, in the meantime, I see two solutions:
>
>   1. If the bug only affects specific applications, we could add an
>      icu4c variant and use that only where needed (icecat, 0ad, etc.).
>
>   2. If the bug manifests itself pervasively we could graft it in
>      ‘master’ (I guess it’s ABI-compatible, right?).
>
> Thoughts?

I prefer the second solution, in case the bug appears in other
applications as well, and I think it is ABI-compatible.  I just sent the
patch for the graft in master.  I'll do the patch for core-updates
later, when master is merged into it.

Message #47 received at 26462 <at> debbugs.gnu.org (full text, mbox):

From: Clément Lassieur <clement <at> lassieur.org>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 26462 <at> debbugs.gnu.org
Subject: Re: bug#26462: [PATCH 1/2] gnu: icu4c: Fix crashes in programs using
 system ICU.
Date: Fri, 14 Apr 2017 16:59:45 +0200

Clément Lassieur <clement <at> lassieur.org> writes:

> Ludovic Courtès <ludo <at> gnu.org> writes:
>
>> Clément Lassieur <clement <at> lassieur.org> skribis:
>>
>>> * gnu/packages/patches/icu4c-reset-keyword-list-iterator.patch: New file.
>>> * gnu/local.mk (dist_patch_DATA): Add it.
>>> * gnu/packages/icu4c.scm (icu4c)[source]: Use it.
>>
>> [...]
>>
>>> +++ b/gnu/packages/patches/icu4c-reset-keyword-list-iterator.patch
>>> @@ -0,0 +1,128 @@
>>> +Copied from upstream: http://bugs.icu-project.org/trac/changeset/39484/.
>>> +
>>> +Paths and line endings have been adapted.
>>
>> Woow, good catch!
>>
>> 1,700 packages depend on icu4c so the update would have to go to
>> ‘core-updates’, but that won’t be merged until several weeks.
>>
>> For ‘master’, in the meantime, I see two solutions:
>>
>>   1. If the bug only affects specific applications, we could add an
>>      icu4c variant and use that only where needed (icecat, 0ad, etc.).
>>
>>   2. If the bug manifests itself pervasively we could graft it in
>>      ‘master’ (I guess it’s ABI-compatible, right?).
>>
>> Thoughts?
>
> I prefer the second solution, in case the bug appears in other
> applications as well, and I think it is ABI-compatible.  I just sent the
> patch for the graft in master.  I'll do the patch for core-updates
> later, when master is merged into it.

But I might be wrong :)  If you think the first solution is better,
don't hesitate to tell me, I'll do another patch.

Message #50 received at 26462 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Clément Lassieur <clement <at> lassieur.org>
Cc: 26462 <at> debbugs.gnu.org
Subject: Re: bug#26462: [PATCH 1/2] gnu: icu4c: Fix crashes in programs using
 system ICU.
Date: Fri, 14 Apr 2017 17:18:30 +0200

Clément Lassieur <clement <at> lassieur.org> skribis:

> Clément Lassieur <clement <at> lassieur.org> writes:
>
>> Ludovic Courtès <ludo <at> gnu.org> writes:
>>
>>> Clément Lassieur <clement <at> lassieur.org> skribis:
>>>
>>>> * gnu/packages/patches/icu4c-reset-keyword-list-iterator.patch: New file.
>>>> * gnu/local.mk (dist_patch_DATA): Add it.
>>>> * gnu/packages/icu4c.scm (icu4c)[source]: Use it.
>>>
>>> [...]
>>>
>>>> +++ b/gnu/packages/patches/icu4c-reset-keyword-list-iterator.patch
>>>> @@ -0,0 +1,128 @@
>>>> +Copied from upstream: http://bugs.icu-project.org/trac/changeset/39484/.
>>>> +
>>>> +Paths and line endings have been adapted.
>>>
>>> Woow, good catch!
>>>
>>> 1,700 packages depend on icu4c so the update would have to go to
>>> ‘core-updates’, but that won’t be merged until several weeks.
>>>
>>> For ‘master’, in the meantime, I see two solutions:
>>>
>>>   1. If the bug only affects specific applications, we could add an
>>>      icu4c variant and use that only where needed (icecat, 0ad, etc.).
>>>
>>>   2. If the bug manifests itself pervasively we could graft it in
>>>      ‘master’ (I guess it’s ABI-compatible, right?).
>>>
>>> Thoughts?
>>
>> I prefer the second solution, in case the bug appears in other
>> applications as well, and I think it is ABI-compatible.  I just sent the
>> patch for the graft in master.  I'll do the patch for core-updates
>> later, when master is merged into it.
>
> But I might be wrong :)  If you think the first solution is better,
> don't hesitate to tell me, I'll do another patch.

No no, I trust you!  I mean, making a graft in master is the safest
approach anyway.

Ludo’.

Message #53 received at 26462 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Clément Lassieur <clement <at> lassieur.org>
Cc: 26462 <at> debbugs.gnu.org
Subject: Re: bug#26462: [PATCH] gnu: icu4c: Fix crashes in programs using
 system ICU.
Date: Sun, 16 Apr 2017 11:54:48 +0200

Hi Clément,

Clément Lassieur <clement <at> lassieur.org> skribis:

> * gnu/packages/patches/icu4c-reset-keyword-list-iterator.patch: New file.
> * gnu/local.mk (dist_patch_DATA): Add it.
> * gnu/packages/icu4c.scm (icu4c)[replacement]: New field.
> (icu4c/fixed): New variable.

Please add “Fixes <http://bugs.gnu.org/26462>.” in the log.

[...]

> +++ b/gnu/packages/patches/icu4c-reset-keyword-list-iterator.patch
> @@ -0,0 +1,128 @@
> +Copied from upstream: http://bugs.icu-project.org/trac/changeset/39484/.
> +
> +Paths and line endings have been adapted.

Could you add a line saying what this fixes?

OK with these changes.  Thanks for taking care of it!

Ludo’.

Message #58 received at 26462-done <at> debbugs.gnu.org (full text, mbox):

From: Clément Lassieur <clement <at> lassieur.org>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 26462-done <at> debbugs.gnu.org
Subject: Re: bug#26462: [PATCH] gnu: icu4c: Fix crashes in programs using
 system ICU.
Date: Mon, 17 Apr 2017 20:45:27 +0200

Ludovic Courtès <ludo <at> gnu.org> writes:

> Hi Clément,
>
> Clément Lassieur <clement <at> lassieur.org> skribis:
>
>> * gnu/packages/patches/icu4c-reset-keyword-list-iterator.patch: New file.
>> * gnu/local.mk (dist_patch_DATA): Add it.
>> * gnu/packages/icu4c.scm (icu4c)[replacement]: New field.
>> (icu4c/fixed): New variable.
>
> Please add “Fixes <http://bugs.gnu.org/26462>.” in the log.
>
> [...]
>
>> +++ b/gnu/packages/patches/icu4c-reset-keyword-list-iterator.patch
>> @@ -0,0 +1,128 @@
>> +Copied from upstream: http://bugs.icu-project.org/trac/changeset/39484/.
>> +
>> +Paths and line endings have been adapted.
>
> Could you add a line saying what this fixes?
>
> OK with these changes.  Thanks for taking care of it!

Done, and pushed.  Thanks for the review!

Clément

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.