Re: SHLWAPI: implement PathIsValidChar{A,W}

[Juan Lang]

Hi Hans, it looks like from values 0x7e onward the character class is
always PATH_CHAR_CLASS_OTHER_VALID.  If you like, for clarity you can fill
in the value through 0x7f, but having a lookup table beyond this isn't
necessary.

Also, that means the data type can be unsigned char, not DWORD.

And, it looks like the return value isn't consistent:

+static BOOL WINAPI PathIsValidCharAW( unsigned char c, DWORD class )
+{
+    return class & SHELL_charclass[c];
+}

Here you mask class with the value in the table..

+BOOL WINAPI PathIsValidCharA( char c, DWORD class )
+{
+    return PathIsValidCharAW( (unsigned char)c, class );
+}

and again here, implicitly..

+BOOL WINAPI PathIsValidCharW( WCHAR c, DWORD class )
+{
+    if (c > 0xff) return PATH_CHAR_CLASS_OTHER_VALID;
+    return PathIsValidCharAW( (unsigned char)c, class );
+}

But for values of c greater than 0xff, you do not mask.  This may be
deliberate, I don't know, but it doesn't look correct.

--Juan

p.s. Have you added me to your spam list yet? ;)

__________________________________ 
Do you Yahoo!? 
Yahoo! Mail - Helps protect you from nasty viruses. 
http://promotions.yahoo.com/new_mail

SHLWAPI: implement PathIsValidChar{A,W}

[Hans Leidekker]

On Sunday 30 January 2005 22:29, Juan Lang wrote:
> Hi Hans, it looks like from values 0x7e onward the character class is
> always PATH_CHAR_CLASS_OTHER_VALID.  If you like, for clarity you can fill
> in the value through 0x7f, but having a lookup table beyond this isn't
> necessary.

Yes I contemplated that but then you reintroduce a conditional in
the ascii case ( if (c > 0x7e) ), which somewhat defeats the purpose
of using a lookup table.
> Also, that means the data type can be unsigned char, not DWORD.

I don't understand you, char class values are DWORD. Do you
mean that, given the number of class values, you could squeeze
them into an unsigned char? Again, then you would have to do more
computation, right? The quest was for speed, not so much for space.
> +BOOL WINAPI PathIsValidCharW( WCHAR c, DWORD class )
> +{
> - if (c > 0xff) return PATH_CHAR_CLASS_OTHER_VALID;
> - return PathIsValidCharAW( (unsigned char)c, class );
>
> +}
>
> But for values of c greater than 0xff, you do not mask.  This may be
> deliberate, I don't know, but it doesn't look correct.

It's deliberate and correct. PATH_CHAR_CLASS_OTHER_VALID *is* a mask so I
do return the same value in both cases. Also remember that the return
type is a BOOL so we could return TRUE everywhere but I chose to mimic
Windows here and return a masked value. The tests show this is the case.
> p.s. Have you added me to your spam list yet? ;)

Of course not! I welcome every opportunity to learn. Thanks for your
constructive criticism.

-Hans