Ximin Luo:
> Hello liblcms2-2 maintainer, just reassigning the bug described below.
> You can reproduce it with $ cd-iccdump <some test file> 
> 
> e.g. from the diffoscope source tree:
> 
> $ cd-iccdump tests/data/test1.icc  | grep 'en_US\|ne_SU'
>   ne_SU:      sRGB [24 bytes]
> [.. etc ..]
> 
> Seems endian-related.
> 
> Reiner Herrmann:
>> On Fri, Dec 09, 2016 at 06:36:53PM +0100, Chris Lamb wrote:
>>> E         -    ne_SU:       sRGB [24 bytes]
>>> E         ?     -  -
>>> E         +    en_US:       sRGB [24 bytes]
>>> E         ?    +  +
>>
>> Just found out that it is caused by liblcms2-2 (2.8-2).
>> After downgrading it to the version in stretch (2.7-1), everything is
>> printed normally.

I think I found the cause. cmsMLUtranslationsCodes() now uses the new
strFrom16() function. The problem is that strFrom16() does not work
under little-endian systems.

  static
  cmsUInt16Number strTo16(const char str[3])
  {
      cmsUInt16Number n = ((cmsUInt16Number) str[0] << 8) | str[1];
  
      return n;  // Always big endian in this case
  }

  static
  void strFrom16(char str[3], cmsUInt16Number n)
  {
      // Assiming this would be aligned
      union {
  
         cmsUInt16Number n;
         char str[2];
         
      } c;
  
      c.n = n;  // Always big endian in this case
  
      str[0] = c.str[0]; str[1] = c.str[1]; str[2] = 0;
  
  }

On a little-endian system strFrom16() wrongly swaps the byte order (even
though the comment says something different). You can easily test this
with the attached minimal test case (see test.c).

I think the easiest solution is the use a machine byte order independent
calculation like in strTo16(). See attached patch.
diff -u a/src/cmsnamed.c b/src/cmsnamed.c
--- a/src/cmsnamed.c
+++ b/src/cmsnamed.c
@@ -192,18 +192,10 @@
 static
 void strFrom16(char str[3], cmsUInt16Number n)
 {
-    // Assiming this would be aligned
-    union {
-
-       cmsUInt16Number n;
-       char str[2];
-       
-    } c;
-
-    c.n = n;  // Always big endian in this case
-
-    str[0] = c.str[0]; str[1] = c.str[1]; str[2] = 0;
-
+    // n is always big endian, see strTo16.
+    str[0] = (n >> 8) & 0xff;
+    str[1] = n & 0xff;
+    str[2] = 0;
 }
 
 // Add an ASCII entry. Do not add any \0 termination (ICC1v43_2010-12.pdf page 61)
#include <stdio.h>
#include <stdint.h>

int main() {
    char s[] = "ab";
    union {
        uint16_t n;
        char s[2];
    } u;
    u.n = s[0] << 8 | s[1];
    printf("%c%c\n", u.s[0], u.s[1]);
}

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to