[PATCH 2/2] winecoreaudio.drv: Don't print 32-bit values as long integers. (Clang)

[Ken Thomases]

On Feb 20, 2016, at 12:47 AM, Dmitry Timoshkov <dmitry at baikal.ru> wrote:
> 
> Charles Davis <cdavis5x at gmail.com> wrote:
> 
>>>> --- a/dlls/winecoreaudio.drv/mmdevdrv.c
>>>> +++ b/dlls/winecoreaudio.drv/mmdevdrv.c
>>>> @@ -385,10 +385,10 @@ HRESULT WINAPI AUDDRV_GetEndpointIDs(EDataFlow
>> flow, WCHAR ***ids,
>>>>         GUID **guids, UINT *num, UINT *def_index)
>>>> {
>>>>     UInt32 devsize, size;
>>>> -    AudioDeviceID *devices;
>>>> +    unsigned int *devices;
>>>>     AudioDeviceID default_id;
>>>>     AudioObjectPropertyAddress addr;
>>>> -    OSStatus sc;
>>>> +    int sc;
>>>>     int i, ndevices;
>>> 
>>> Hi, Charles. I usually don't review patches but it feels weird to me
>>> that you are changing the type of the variable because
>>> AudioObjectGetPropertyData expects an AudioDeviceID and not an
>>> unsigned int. Even if they are the same I believe using the correct
>>> type is better. The same for OSStatus which is the return for
>>> AudioObjectGetPropertyData.
>> It actually feels weird to me, too. Originally, I cast them in the debug
>> prints, but when I did that in advapi32, AJ wanted me to just change the
>> variable type. So that's what I'm doing now.
> 
> What's wrong with using correct printf format specifier instead?

Unfortunately, for historical compatibility reasons, these types are defined as (unsigned) long for 32-bit but (unsigned) int for 64-bit.  So, there's no one correct format specifier.  Well, at least not a cross-platform one supported by Wine's logging functions.

I'm ambivalent about using int instead of OSStatus, because I think it doesn't make the code significantly more confusing.  However, using unsigned int instead of AudioDeviceID seems like it loses important semantic information.  It's also likely to backslide as new code is written (assuming others habitually follow the API, as I would).

Also, stuff like seen in the hunk above where the type of "devices" is changed but the type of "default_id" is not, just because one is used in logging and the other is not, is bothersome.  If a new logging statement is added that starts logging default_id, will we have to change its type?  That's no way to live. ;)

Alexandre, is this really preferable to casting in the logging statements?

-Ken