64780 - Problems with the collation sortkeys

Status: VERIFIED INVALID

(Core :: Internationalization, defect)

Description

[Mike Kaply [:mkaply]]

We are working on sorting on OS/2, and we have found some bugs in the collation 
stuff.

In nsICollation.h at 
http://lxr.mozilla.org/seamonkey/source/intl/locale/public/nsICollation.h#75

it is indicated that GetSortKeyLen returns a byte length and that 
CreateRawSortKey takes a byte length.

However, in actual usage, some calls assume byte length, and other assume 
UniChar length.

First look at:

http://lxr.mozilla.org/seamonkey/source/intl/locale/src/nsCollation.cpp#108

According to the spec, this code improperly allocates aKey1 as aLength1 
PRUInt8's

In:

http://lxr.mozilla.org/seamonkey/source/mailnews/addrbook/src/nsAddrDatabase.cpp
#3968

The sort key is allocated as bytes indicated in the header file.

So there is inconsistency in the code.

In addition, due to an inadequate C spec, I believe the unix GetSortKeyLen 
might be broke as well.

See:

http://www.opennc.org/onlinepubs/7908799/xsh/strxfrm.html

Notice that the return value really doesn't say whether it is length of string 
in unicode characters or length of string in bytes.

So the unix implementation of GetSortKeyLen in nsCollationUnix returns this len 
as the length of the key. Is it bytes or Unicode characters?

I'm not sure what the correct behavior is, but I do know that there are bugs 
here, because we have two code paths that do completely different things.

Comment 1

[nhottanscp]

I don't understand about the problem, PRUInt8 is unsigned char.

Comment 2

[Mike Kaply [:mkaply]]

I must be losing my mind.

I was reading those PRUint8's as PRUInt16's for some reason.

So GetSortKeyLen does return bytes that should be allocated.

And if inside CreateRawSortKey, we need the length of the key, we should just 
call GetSortKeyLen again.

And we are sure strxfrm returns the length in bytes?

Comment 3

[nhottanscp]

strxfrm generates a key in char*, character length is same as byte lenght.

Comment 4

[Teruko Kobayashi]

Verified.