[2/5] dinput: SetActionMap saving simple configurations to an .ini file (try 2)

Vitaliy Margolen wine-devel at kievinfo.com
Mon Jan 9 20:49:43 CST 2012


On 01/09/2012 10:18 AM, Lucas Fialho Zawacki wrote:
> From: Lucas Fialho Zawacki<lfzawacki at gmail.com>
>
> +static BOOL _write_private_profile_intW(const char *format, WCHAR* section, WCHAR* key, int value, WCHAR* file)
I don't think this is such a good idea to mix ASCII and WCHAR parameters.

> +    static WCHAR path[] = {
> +        '%','C','o','m','m','o','n','P','r','o','g','r','a','m','F','i','l','e','s','%','\\',
> +        'D','i','r','e','c','t','X','\\',
> +        'D','i','r','e','c','t','I','n','p','u','t','\\',
> +        'U','s','e','r',' ','M','a','p','s','\0'};
Why do you think it should be there in the first place?

> +    expanded_path = HeapAlloc(GetProcessHeap(), 0, sizeof(WCHAR)*expanded_path_size);
No error checking.

> +static HRESULT _save_mapping_settings(IDirectInputDevice8W *iface, LPDIACTIONFORMATW lpdiaf, LPCWSTR lpszUsername)
> +{
> +    static WCHAR mapexists[] = {'M','a','p','E','x','i','s','t','s','\0'};
> +    static WCHAR numactions[] = {'N','u','m','A','c','t','i','o','n','\0'};
Should be "static const WCHAR".

> +    IDirectInputDevice8_GetDeviceInfo(iface,&didev);
No error checking.

> +    va_start(args, format);
> +    while (1)
> +    {
> +        buffer = HeapAlloc(GetProcessHeap(), 0, size);
> +        if (buffer == NULL)
> +            break;
> +        n = vsnprintfW(buffer, size, formatW, args);
> +        if (n == -1)
> +            size *= 2;
> +        else if (n>= size)
> +            size = n + 1;
> +        else
> +            break;
> +        HeapFree(GetProcessHeap(), 0, buffer);
> +    }
> +    va_end(args);
You can't do this. It seems to be you have not tested it with initial buffer 
too small. You have to do va_start & va_end every time you pass "args" to 
another function.

> +
> +    if (buffer == NULL) return NULL;
> +    ret = HeapReAlloc(GetProcessHeap(), 0, buffer, sizeof(WCHAR)*(strlenW(buffer)+1) );
> +    if (ret == NULL) ret = buffer;
Why reallocate? It's pointless.

> +extern WCHAR* _get_mapping_path(const WCHAR *device, const WCHAR *username) DECLSPEC_HIDDEN;
Would you please drop leading underscore from all of your function names?

Vitaliy.



More information about the wine-devel mailing list