Re: usp10: Resend - Add funtionality for ScriptStringAnalyse

[Vitaliy Margolen]

Saturday, July 29, 2006, 8:34:31 PM, Jeff L wrote:
> A resend with after a shortening of the variable names, fixing the 
> handelling of multiple scripts and a slight rework to remove tests 
> before HeapFree calls and
> meanigful variables.

You need to make few more changes:
> - lstrcpyW(lf.lfFaceName, (WCHAR *) "Courier");
> - lstrcpyW(lf.lfFaceName, (WCHAR *) L"Courier");

This is not portable and should be something like:
const WCHAR courierW[] = {'C','o','u','r','i','e','r',0};
lstrcpyW(lf.lfFaceName, courierW);
> - WCHAR           teststr[6] = {'T', 'e', 's', 't', 'a', '\0'};
> - WCHAR           teststr[6] = {'T', 'e', 's', 't', 'b', '\0'};

Drop the lenght:
WCHAR teststr[] = {'T', 'e', 's', 't', 'b', '\0'};
> - hr = ScriptStringAnalyse( NULL, (void*)pString, cString, cGlyphs, iCharset, dwFlags,

I don't think you need to cast a pointer into (void *) they all already are.
> -/*    Commented code it pending new code in ScriptStringAnalysis */
> -/*    ok(hr == S_OK, "ScriptStringAnalyse Stub should return S_OK not %08x\n", (unsigned int) hr);*/
> -/*    ok(pssa != NULL, "ScriptStringAnalyse pssa should not be NULL\n");*/

Does it crash that you commented it? If it doesn't crash but just doesn't work
do this instead:
todo_wine {
ok(failing_test);
ok(failing_test);
}
> - ok(hr == S_OK, "ScriptStringAnalyse Stub should return S_OK not %08x\n", (unsigned int) hr);

That's not correct. Don't cast hr, use proper format instead:
ok(hr == S_OK, "ScriptStringAnalyse Stub should return S_OK not %08lx\n", hr);
> +typedef struct script_string_analysis_ptrs
> +{
> -
>   int     cGlyphs;
> -
>   SCRIPT_VISATTR *psva;

Please use the same format as the rest of the file / recommended Wine format -
in this case 4 space indent.
> - if  (1 > cString || NULL == pString) {
> -
>    TRACE("cString %d, pString(%p)\n", cString, pString);
> -
>    return E_INVALIDARG;
> - }

Can you use something that every other person would understand?
    if  (cString < 1 || !pString) {
Please use WARN if you want to indicate app's errors:
        WARN("cString %d, pString(%p)\n", cString, pString);
But in particular case, I don't see a need for this. You already have
all dumping TRACE right at the beginning of the function. It will log
any errors as well.

Vitaliy Margolen

usp10: Resend - Add funtionality for ScriptStringAnalyse

[Jeff Latimer]

Vitaliy Margolen wrote:

This one implements the checks described in the API. I think that we 
need to reproduce those checks but I can make the message more clear though.

Jeff
> Can you use something that every other person would understand?
>    if  (cString < 1 || !pString) {
> Please use WARN if you want to indicate app's errors:
>        WARN("cString %d, pString(%p)\n", cString, pString);
> But in particular case, I don't see a need for this. You already have
> all dumping TRACE right at the beginning of the function. It will log
> any errors as well.
>
> Vitaliy Margolen