254252 - nsCRT::BufferHashCode has two variants, and only one user, HashCodeAsUTF8 has no users

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Mikael Parknert]

nsCRT::BufferHashCode has two variants, and only one consumer, one
could be removed.

The consumer 
(http://lxr.mozilla.org/seamonkey/source/content/shared/public/nsHTMLValue.h#140
)
uses the char*-version but maybe it should use the PRUnichar*-version? Anyway, 
one of these variants could be removed.

Comment 1

[Mikael Parknert]

It also looks like HashCodeAsUTF8 has no consumer at all.

http://lxr.mozilla.org/seamonkey/search?string=HashCodeAsUTF8

Comment 2

[Mikael Parknert]

replace consumer with user in previous comments

Comment 3

[Mikael Parknert]

Attachment: patch

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 155374 [details] [diff] [review]
patch

Looks fine to me if darin's OK with it.  sr=bzbarsky.

Comment 5

[Darin Fisher]

Comment on attachment 155374 [details] [diff] [review]
patch

Hmm... changing to the wide version of BufferHashCode appears to change the
hashing algorithm since it will be combining 16-bit units instead of 8-bit
units, or am I mistaken?  I can't see any reason why that should matter, unless
some other code elsewhere is using PL_HashString or some other implementation
of the same algorithm as the narrow version of BufferHashCode.	Are we 100%
sure this isn't going to cause wierd side-effects?  I find it odd that jkeiser
added the wide version of BufferHashCode at the same time that he added this
HashCode function.  Do you think he just mistakenly used the narrow version to
begin with?  (It did go through 3 reviewers... chalk it up to a typo or could
there be some real reason behind his choice?)

Also, seems like a shame to throw away such good code for computing the UTF-8
equivalent hash on a UTF-16 string.  Maybe we should just "#if 0" the code in
case someone needs it again?

r=darin

Comment 6

[timeless]

mikael: do you need someone to commit this?

Comment 7

[timeless]

Comment on attachment 155374 [details] [diff] [review]
patch

mozilla/content/base/src/nsHTMLValue.h	3.42
mozilla/xpcom/ds/nsCRT.cpp	3.56
mozilla/xpcom/ds/nsCRT.h	3.72

Comment 8

[Johnny Stenback (:jst)]

Argh! I'm using HashCodeAsUTF8(). timeless, it'd been nice if you'd followed
darin's sr commeints before checking this in...