July 2016 - Wine-Devel - list.winehq.org

Re: joy.cpl: Display the correct number of button boxes during test
by DavidL July 22, 2016

July 22, 2016

I noticed this as well - thanks! On Friday, July 22, 2016, Bruno Jesus <00cpxxx(a)gmail.com> wrote: > After testing with an 8 button only joystick I noticed that currently > the program displays 9 buttons. Then I tested with an PS2 + usb > adapter controller and noticed that it is also off by one. I believe > nobody notices this because the gamepads have too many buttons > nowadays and you never test all of them. > > Signed-off-by: Bruno Jesus <00cpxxx(a)gmail.com <javascript:;>> >

1 0

Re: [PATCH] ws2_32: Implement InetNtopW
by Bruno Jesus July 22, 2016

July 22, 2016

On Fri, Jul 22, 2016 at 5:16 PM, Philipp Hoppermann <plata(a)mailbox.org> wrote: > Fixes https://bugs.winehq.org/show_bug.cgi?id=40999 Thanks, I have written some tests to back your changes and will send soon, I also noticed a mistake in my review of your patch that I'll fix and send a new version.

1 0

Re: [PATCH 2/2] ws2_32: Fix implementation of InetNtopW.
by Bruno Jesus July 22, 2016

July 22, 2016

On Fri, Jul 22, 2016 at 9:04 AM, Philipp Hoppermann <plata(a)mailbox.org> wrote: > Replaced call to inet_ntop by more elaborate solution. Looks much better, thanks. See the comments below. > Signed-off-by: Philipp Hoppermann <plata(a)mailbox.org> > --- > dlls/ws2_32/socket.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 68 insertions(+), 4 deletions(-) > > diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c > index 0622060..2045163 100644 > --- a/dlls/ws2_32/socket.c > +++ b/dlls/ws2_32/socket.c > @@ -7934,11 +7934,75 @@ INT WINAPI InetPtonW(INT family, PCWSTR addr, PVOID buffer) > } > > /*********************************************************************** > -* InetNtopW (WS2_32.@) > -*/ > -PCWSTR WINAPI InetNtopW(INT family, PVOID addr, PWSTR stringBuf, SIZE_T stringBufSize) > + * InetNtopW (WS2_32.@) > + * > + * Get string representation of IP address. > + * > + * PARAMS > + * family [I] AF_INET or AF_INET6 > + * addr [I] IP address in byte > + * buffer [O] IP address as string > + * buffer_len [O] length of the buffer in characters > + * > + * RETURNS > + * Success: IP address as string > + * Failure: NULL. Use GetLastError() to find the error cause. > + * > + * Error codes: > + * WSAEAFNOSUPPORT > + * - family is not AF_INET or AF_INET6 > + * ERROR_INVALID_PARAMETER > + * - buffer is NULL > + * - buffer_len is 0 > + * - buffer_len is too small > + * (i.e. < 16 for AF_INET or < 46 for AF_INET6) > + * > + */ Usually only overly complex functions need comments, which is not the case here so you can drop them. As you can see in the file most almost no other function has comments like this. > +PCWSTR WINAPI InetNtopW(INT family, PVOID addr, PWSTR buffer, SIZE_T buffer_len) > { > - return inet_ntop(family, addr, stringBuf, stringBufSize); > + char *bufferA; > + > + TRACE("family %d, addr %s, buffer (%p), buffer_len %lu\n", family, debugstr_w(addr), buffer, (unsigned long) buffer_len); WS_inet_ntop traces buffer_len without the cast using %ld, I guess it is better to follow its example and avoid the cast. > + // check family // comments are not allowed, please use /* */. But most of them are obvious like "check if buffer is NULL" so they can be dropped. > + if ((family != AF_INET) && (family != AF_INET6)) > + { > + SetLastError(WSAEAFNOSUPPORT); > + return NULL; > + } > + > + // check if buffer is NULL > + if (buffer == NULL) > + { > + SetLastError(ERROR_INVALID_PARAMETER); > + return NULL; > + } > + > + // check buffer_len > + if ((family == AF_INET && buffer_len < 16) || (family == AF_INET6 && buffer_len < 46)) > + { > + SetLastError(ERROR_INVALID_PARAMETER); > + return NULL; > + } WS_inet_ntop already does some error checking, there is no need to manually test again, also we trust the system inet_ntop call to deal with buffer_len check. > + // allocate memory for the string representation in char > + if (!(bufferA = HeapAlloc(GetProcessHeap(), 0, buffer_len*sizeof(WCHAR)))) > + { > + SetLastError(WSA_NOT_ENOUGH_MEMORY); > + return NULL; > + } You are doubling the required amount of memory, buffer_len should be already in characters, so no need to multiply. > + if (WS_inet_ntop(family, addr, bufferA, buffer_len*sizeof(WCHAR)) != NULL) Again no need to multiply. > + { > + // convert to wchar > + MultiByteToWideChar(CP_ACP, 0, bufferA, buffer_len*sizeof(WCHAR), buffer, buffer_len); Use: MultiByteToWideChar(CP_ACP, 0, bufferA, -1, buffer, buffer_len); > + > + HeapFree(GetProcessHeap(), 0, bufferA); > + return buffer; > + } else > + { > + return NULL; You are leaking bufferA in the else case, use a variable to store the results of WS_inet_ntop and return it instead so you can always HeapFree before the return and then there is no need for else. Best wishes, Bruno

1 0

Re: [PATCH 1/2] ws2_32: Implement InetNtopW.
by Bruno Jesus July 22, 2016

July 22, 2016

On Fri, Jul 22, 2016 at 9:04 AM, Philipp Hoppermann <plata(a)mailbox.org> wrote: > Fixes "unimplemented function WS2_32.dll.InetNtopW" > > At the moment, this fix just calls inet_ntop It is not necessary to split the patch, specially because this patch is broken because of the char/WCHAR thing. Merge the spec changes in the 2/2 patch and see my other comments there.

1 0

Re: include: Improve include directive in d3dtypes.h.
by Henri Verbeet July 22, 2016

July 22, 2016

On 22 July 2016 at 15:31, Andrey Gusev <andrey.goosev(a)gmail.com> wrote: > -#include <d3dvec.inl> > +#include "d3dvec.inl" Is that really an improvement? I'd argue these are system headers and angle brackets are correct. Of course in that case there are a couple of places where we'd want to change things in the other direction. Whether that's worth the trouble may be a different question.

1 0

Re: [v3 1/1] winejoystick.drv: Sort virtual joysticks by name on the Mac
by Ken Thomases July 22, 2016

July 22, 2016

Hi, On Jul 20, 2016, at 2:15 AM, David Lawrie <david.dljunk(a)gmail.com> wrote: > > +static CFStringRef copy_device_name(IOHIDDeviceRef device) > +{ > + CFTypeRef ref_name, ref_man; > + CFStringRef name; > + long prodID; I prefer variables to be declared at the narrowest scope in which they're used. > + > + if (device) > + { > + assert(IOHIDDeviceGetTypeID() == CFGetTypeID(device)); > + > + ref_name = IOHIDDeviceGetProperty(device, CFSTR(kIOHIDProductKey)); > + > + if (ref_name && CFStringGetTypeID() == CFGetTypeID(ref_name)) > + name = CFStringCreateCopy(kCFAllocatorDefault,ref_name); > + else > + { > + ref_man = IOHIDDeviceGetProperty(device, CFSTR(kIOHIDManufacturerKey)); I think it's simpler to use kIOHIDVendorIDKey with get_device_property_long(). Judging by the HID Calibrator code, kIOHIDManufacturerKey is not reliably available, but kIOHIDVendorIDKey should be. It's also pretty fundamental to USB/HID. > + prodID = get_device_property_long(device, CFSTR(kIOHIDProductIDKey)); > + > + if(ref_man && CFStringGetTypeID() == CFGetTypeID(ref_man)) > + name = CFStringCreateWithFormat(kCFAllocatorDefault, NULL, CFSTR("%@ %lu"), ref_man, prodID); Better to format vendor and product IDs using "0x%04lx" since that's how they are usually represented (e.g. within HID Calibrator windows and System Profiler). (Yes, I realize that Copy_DeviceName() which I cited previously formats them as decimal.) Thanks for persevering in the face of my picky-ness. :) -Ken

2 1

Re: [3/6] msvcirt: Forward signed char exports to char functions directly
by Alexandre Julliard July 22, 2016

July 22, 2016

"Iván Matellanes" <matellanesivan(a)gmail.com> writes: > signed char and char are equivalent in Visual C++, so specific functions for signed char are unnecessary. Note that you can't assume that 'char' is signed for the Unix compiler. If it makes a difference, you'd have to specify signed char explicitly. Otherwise, you could as well you the same functions for the unsigned case too. -- Alexandre Julliard julliard(a)winehq.org

2 1

Re: [v2 PATCH 4/4] hnetcfg: Add initial tests
by Marvin July 22, 2016

July 22, 2016

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=24452 Your paranoid android. === build (build) === Make failed

2 1

Re: [PATCH] dinput : use vendor and product ids to generate product GUID like it's done on Windows
by Bruno Jesus July 22, 2016

July 22, 2016

On Thu, Jul 21, 2016 at 7:08 PM, Corentin Rossignon <corossig(a)gmail.com> wrote: > Signed-off-by: Corentin Rossignon <corossig(a)gmail.com> > --- > dlls/dinput/joystick_linuxinput.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/dlls/dinput/joystick_linuxinput.c b/dlls/dinput/joystick_linuxinput.c > index 86f12d0..e5172b6 100644 > --- a/dlls/dinput/joystick_linuxinput.c > +++ b/dlls/dinput/joystick_linuxinput.c > @@ -321,13 +321,19 @@ static void find_joydevs(void) > static void fill_joystick_dideviceinstanceA(LPDIDEVICEINSTANCEA lpddi, DWORD version, int id) > { > DWORD dwSize = lpddi->dwSize; > + // Data1 is concatenation of productid and vendorid, Data4 seems constant > + GUID product_guid = { ((joydevs[id].product_id)<<16)+joydevs[id].vendor_id, > + 0x0000, > + 0x0000, > + {0x00, 0x00, 0x50, 0x49, 0x44, 0x56, 0x49, 0x44} }; > + Hi, I'm not familiar with direct input code but I can say that // comments are not allowed, please use /* */ and send a try 2. Is there any easy way to test this patch? Like a simple app that displays such information so I can help testing in Windows and Wine?

2 2

Re: [v3 1/1] dinput: Sort virtual joysticks by name on the Mac
by Ken Thomases July 22, 2016

July 22, 2016

On Jul 20, 2016, at 2:15 AM, David Lawrie <david.dljunk(a)gmail.com> wrote: > > Tested on OS X 10.10.5. > > Signed-off-by: David Lawrie <david.dljunk(a)gmail.com> > --- > dlls/dinput/joystick_osx.c | 50 +++++++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 47 insertions(+), 3 deletions(-) Same comments as for the winejoystick version. -Ken

1 0