[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-core-devel
Subject: Re: Review Request: little faster sycoca
From: Josef Weidendorfer <Josef.Weidendorfer () gmx ! de>
Date: 2011-09-29 8:36:03
Message-ID: 201109291036.03502.Josef.Weidendorfer () gmx ! de
[Download RAW message or body]
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
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic