Re: [PATCH 6/6] ole32: Implement MkParseDisplayName.

[Juan Lang]

Hi Rob,

+/* find a character from a set in reverse without the string having to be
null-terminated */
+static inline WCHAR *memrpbrkW(const WCHAR *ptr, size_t n, const WCHAR
*accept)
+{
+    const WCHAR *end, *ret = NULL;
+    for (end = ptr + n; ptr < end; ptr++) if (strchrW(accept, *ptr)) ret
= ptr;
+    return (WCHAR *)ret;
+}

Maybe it's because I'm dense and I don't particularly like three levels of
indent on a single line, but this doesn't seem to do what I'd expect.  The
for loop doesn't quit when ret is assigned, thus, this will always find
the last character, from left to right, from accept in ptr, rather than
the first.

Don't you mean "ptr < end && !ret"?  And in what sense is this "in
reverse"?
--Juan

____________________________________________________________________________________
Bored stiff? Loosen up... 
Download and play hundreds of games for free on Yahoo! Games.
http://games.yahoo.com/games/front

[PATCH 6/6] ole32: Implement MkParseDisplayName.

[Robert Shearman]

Juan Lang wrote:
> +/* find a character from a set in reverse without the string having to be
> null-terminated */
> +static inline WCHAR *memrpbrkW(const WCHAR *ptr, size_t n, const WCHAR
> *accept)
> +{
> - const WCHAR *end, *ret = NULL;
> - for (end = ptr + n; ptr < end; ptr++) if (strchrW(accept, *ptr)) ret
>
> = ptr;
> - return (WCHAR *)ret;
>
> +}
>
> Maybe it's because I'm dense and I don't particularly like three levels of
> indent on a single line, but this doesn't seem to do what I'd expect.  The
> for loop doesn't quit when ret is assigned, thus, this will always find
> the last character, from left to right, from accept in ptr, rather than
> the first.

I tested this function so I'm fairly sure it's correct. It's also based 
on the code of several functions merged into one from 
include/wine/unicode.h (including strrchrW). Perhaps it would be better 
to rework the function for efficiency.
I'm pretty sure it does the right thing though, as can be shown in the 
following example:
ptr: "abcdefg", size: 6, accept: "bdg"

end = ptr + 8 (end is "g")
ptr < end = TRUE
strchrW("bdg", 'a') = NULL
ptr++ (ptr is now "bcdefg")
ptr < end = TRUE
strchrW("bdg", 'b') = "bdg"
ret = ptr (ret is now "bcdefg")
ptr++ (ptr is now "cdefghi")
ptr < end = TRUE
strchrW("bdg", 'c') = NULL
ptr++ (ptr is now "defg")
ptr < end = TRUE
strchrW("bdg", 'd') = "dg"
ret = ptr (ret is now "defg")
ptr++ (ptr is now "efg")
ptr < end = TRUE
strchrW("bdg", 'e') = NULL
ptr++ (ptr is now "fg")
ptr < end = TRUE
strchrW("bdg", 'f') = NULL
ptr++ (ptr is now "g")
ptr < end = FALSE
return ret (ret is "defg")
> Don't you mean "ptr < end && !ret"?

I suppose that could be done for safety, but in this case it doesn't a 
matter as I've calculated size using strlenW.
> And in what sense is this "in
> reverse"?

It returns the position of the last character in the input string that 
matches a character in accept.

[PATCH 6/6] ole32: Implement MkParseDisplayName.

[Juan Lang]

Hi Rob,
> I tested this function so I'm fairly sure it's correct.

Yes, that's why I said I was dense ;)
> Perhaps it would be better to rework the function for efficiency.
> I'm pretty sure it does the right thing though...

Thanks for the explanation.  Yes, you're right.  It appears to do what it
says it does, and it has the (dubious) benefit of having predictable
runtime:  its runtime is never better than the worst case.

--Juan

____________________________________________________________________________________
It's here! Your new message!  
Get new email alerts with the free Yahoo! Toolbar.
http://tools.search.yahoo.com/toolbar/features/mail/