From kde-core-devel  Thu Sep 29 08:36:03 2011
From: Josef Weidendorfer <Josef.Weidendorfer () gmx ! de>
Date: Thu, 29 Sep 2011 08:36:03 +0000
To: kde-core-devel
Subject: Re: Review Request: little faster sycoca
Message-Id: <201109291036.03502.Josef.Weidendorfer () gmx ! de>
X-MARC-Message: https://marc.info/?l=kde-core-devel&m=131728538826476

On Wednesday 28 September 2011, Rolf Eike Beer wrote:
> What happens if inPos is -1? pos becomes 0 then. Then we iterate over the 
> whole list just to do "if (... && pos != 0)" which will never be true. So for 
> this case (inPos == -1) the whole function can be avoided at all as it will 
> never return anything else but 0. So the initial check of the function should 
> IMHO be:
> 
>     if (inPos == 0 || inPos == -1)
>         return 0;

You are right.

I assume this actually is a off-by-one bug, as the comment above says:

 // The hasList is used for hashing:
 //  hashList = (-2, 1, 3) means that the hash key comes from
 //  the 2nd character from the right, then the 1st from the left, then the
 //  3rd from the left.

If -2 should be 2nd from right, then -1 should be first from right, ie.
entry->key[entry.length-1]. The current version of the patch fixes this
in calcDiversity(...).

But KSycocaDict::Private::hashKey(...) should do exactly the same, otherwise
the hashing uses other positions than found by heavy work in calcDiversity(...).

Jaime, I think the current patch is wrong here for hashKey():
It just should be "pos = -pos" in line 235, and should add "if (pos==0) continue;"
before line 234 to match calcDiversity(). Or do I miss something?

Josef