Re: riched20: Implements FR_MATCHCASE for EM_FINDTEXT
On Tue, 21 Feb 2006 14:08:38 -0800 Lei Zhang <leiz(a)ucla.edu> wrote:
On Tuesday 21 February 2006 12:42 pm, you wrote:
On Tue, 21 Feb 2006 10:19:01 -0800
Lei Zhang <leiz(a)ucla.edu> wrote:
- while (pCurItem && pCurItem->member.run.strText->szData[nCurEnd - nMatched - 1] == text[nLen - nMatched - 1]) + while (ME_CharCompare(pCurItem->member.run.strText->szData[nCurEnd - nMatched - 1], text[nLen - nMatched - 1], (flags & FR_MATCHCASE)))
You've dropped pCurItem NULL check here. Probably just a typo...
-- Ph.
Hi Phil,
Ya, it's a typo. Thanks for catching it.
Could you kindly fix it then? As this patch was accepted unfixed. -- Ph.
Phil Krylov <phil(a)newstar.rinet.ru> writes:
On Tue, 21 Feb 2006 14:08:38 -0800 Lei Zhang <leiz(a)ucla.edu> wrote:
Hi Phil,
Ya, it's a typo. Thanks for catching it.
Could you kindly fix it then? As this patch was accepted unfixed.
I accepted the patch because I don't see how pCurItem could be NULL in that loop (or rather, it could be NULL, but in that case we'll crash before coming back to the loop test). Am I missing something? -- Alexandre Julliard julliard(a)winehq.org
On Fri, 24 Feb 2006 12:24:45 +0100 Alexandre Julliard <julliard(a)winehq.org> wrote:
Phil Krylov <phil(a)newstar.rinet.ru> writes:
On Tue, 21 Feb 2006 14:08:38 -0800 Lei Zhang <leiz(a)ucla.edu> wrote:
Hi Phil,
Ya, it's a typo. Thanks for catching it.
Could you kindly fix it then? As this patch was accepted unfixed.
I accepted the patch because I don't see how pCurItem could be NULL in that loop (or rather, it could be NULL, but in that case we'll crash before coming back to the loop test). Am I missing something?
Yes, you're right. It can't be NULL because of an earlier test: para->member.para.nCharOfs + item->member.run.nCharOfs + nEnd - nLen >= nMin But this is very inevident. -- Ph.
On Friday 24 February 2006 04:39 am, Phil Krylov wrote:
On Fri, 24 Feb 2006 12:24:45 +0100
Alexandre Julliard <julliard(a)winehq.org> wrote:
Phil Krylov <phil(a)newstar.rinet.ru> writes:
On Tue, 21 Feb 2006 14:08:38 -0800
Lei Zhang <leiz(a)ucla.edu> wrote:
Hi Phil,
Ya, it's a typo. Thanks for catching it.
Could you kindly fix it then? As this patch was accepted unfixed.
I accepted the patch because I don't see how pCurItem could be NULL in that loop (or rather, it could be NULL, but in that case we'll crash before coming back to the loop test). Am I missing something?
Yes, you're right. It can't be NULL because of an earlier test:
para->member.para.nCharOfs + item->member.run.nCharOfs + nEnd - nLen
= nMin
But this is very inevident.
-- Ph.
Hi Phil, Alex Yes, it can become NULL, here's how: Inside while (ME_CharCompare(pCurItem->.... ) the last if() block assigns a new value to pCurItem, which may be NULL. Sorry I didn't patch this earlier, I'll do it today. - Lei
On Fri, 24 Feb 2006 10:48:47 -0800 Lei Zhang <leiz(a)ucla.edu> wrote:
On Friday 24 February 2006 04:39 am, Phil Krylov wrote:
On Fri, 24 Feb 2006 12:24:45 +0100
Alexandre Julliard <julliard(a)winehq.org> wrote:
Phil Krylov <phil(a)newstar.rinet.ru> writes:
On Tue, 21 Feb 2006 14:08:38 -0800
Lei Zhang <leiz(a)ucla.edu> wrote:
Hi Phil,
Ya, it's a typo. Thanks for catching it.
Could you kindly fix it then? As this patch was accepted unfixed.
I accepted the patch because I don't see how pCurItem could be NULL in that loop (or rather, it could be NULL, but in that case we'll crash before coming back to the loop test). Am I missing something?
Yes, you're right. It can't be NULL because of an earlier test:
para->member.para.nCharOfs + item->member.run.nCharOfs + nEnd - nLen
= nMin
But this is very inevident.
-- Ph.
Hi Phil, Alex
Yes, it can become NULL, here's how:
Inside while (ME_CharCompare(pCurItem->.... ) the last if() block assigns a new value to pCurItem, which may be NULL.
It may but it won't... ME_FindItemBack(pCurItem) will return NULL only if pCurItem is the first text run in the document. But the if() condition (nCurEnd - nMatched == 0) is TRUE only if the search string is longer than nCurEnd. This does not happen for the first document run because of the condition of the exterior loop which checks if we have enough characters for comparison. --Ph.
On Friday 24 February 2006 11:10 am, Phil Krylov wrote:
On Fri, 24 Feb 2006 10:48:47 -0800
Lei Zhang <leiz(a)ucla.edu> wrote:
On Friday 24 February 2006 04:39 am, Phil Krylov wrote:
On Fri, 24 Feb 2006 12:24:45 +0100
Alexandre Julliard <julliard(a)winehq.org> wrote:
Phil Krylov <phil(a)newstar.rinet.ru> writes:
On Tue, 21 Feb 2006 14:08:38 -0800
Lei Zhang <leiz(a)ucla.edu> wrote:
Hi Phil,
Ya, it's a typo. Thanks for catching it.
Could you kindly fix it then? As this patch was accepted unfixed.
I accepted the patch because I don't see how pCurItem could be NULL in that loop (or rather, it could be NULL, but in that case we'll crash before coming back to the loop test). Am I missing something?
Yes, you're right. It can't be NULL because of an earlier test:
para->member.para.nCharOfs + item->member.run.nCharOfs + nEnd - nLen
= nMin
But this is very inevident.
-- Ph.
Hi Phil, Alex
Yes, it can become NULL, here's how:
Inside while (ME_CharCompare(pCurItem->.... ) the last if() block assigns a new value to pCurItem, which may be NULL.
It may but it won't... ME_FindItemBack(pCurItem) will return NULL only if pCurItem is the first text run in the document. But the if() condition (nCurEnd - nMatched == 0) is TRUE only if the search string is longer than nCurEnd. This does not happen for the first document run because of the condition of the exterior loop which checks if we have enough characters for comparison.
--Ph.
Well, in any case, I'm still leaning toward putting in the test for NULL, both as a sanity check and to so we don't rely on ME_FindItemBack's behavior current. The test is not expensive either, so unless there's a serious objection, I'll submit a patch that tests for NULL. - Lei
participants (3)
-
Alexandre Julliard -
Lei Zhang -
Phil Krylov