Review Request: little faster sycoca
Josef Weidendorfer
Josef.Weidendorfer at gmx.de
Thu Sep 29 09:36:03 BST 2011
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
More information about the kde-core-devel
mailing list