September 2013 - Wine-Devel

Re: wldap32: Use BOOL type where appropriate

by Rico Schüller

On 01.10.2013 00:06, Frédéric Delanoy wrote: > --- > dlls/wldap32/init.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/dlls/wldap32/init.c b/dlls/wldap32/init.c > index b5aed58..54b5b24 100644 > --- a/dlls/wldap32/init.c > +++ b/dlls/wldap32/init.c > @@ -113,13 +113,13 @@ oom: > } > > /* Determine if a URL starts with a known LDAP scheme */ > -static int has_ldap_scheme( char *url ) > +static BOOL has_ldap_scheme( char *url ) > { > if (!strncasecmp( url, "ldap://", 7 ) || > !strncasecmp( url, "ldaps://", 8 ) || > !strncasecmp( url, "ldapi://", 8 ) || > - !strncasecmp( url, "cldap://", 8 )) return 1; > - return 0; > + !strncasecmp( url, "cldap://", 8 )) return TRUE; > + return FALSE; > } > > /* Flatten an array of hostnames into a space separated string of URLs. > You probably may return the "value" of the if statement directly instead of returning TRUE / FALSE. Cheers Rico

Sept. 30, 2013

Re: [4/5] ntdll: Make asynchronous NtWriteFile on a disk file always return STATUS_PENDING.

by Alexandre Julliard

Dmitry Timoshkov <dmitry(a)baikal.ru> writes: > --- > dlls/ntdll/file.c | 3 ++- > dlls/ntdll/tests/file.c | 5 +++-- > 2 files changed, 5 insertions(+), 3 deletions(-) I don't see the point. Do you actually have an app that depends on this? -- Alexandre Julliard julliard(a)winehq.org

Sept. 30, 2013

Re: [3/3] ws2_32: Implement WSASendMsg() (try 2)

by Bruno Jesus

On Mon, Sep 30, 2013 at 6:32 AM, Alexandre Julliard <julliard(a)winehq.org> wrote: > > Bruno Jesus <00cpxxx(a)gmail.com> writes: > > > @@ -2427,6 +2427,46 @@ static void WINAPI WS2_GetAcceptExSockaddrs(PVOID buffer, DWORD data_size, DWORD > > } > > > > /*********************************************************************** > > + * WSASendMsg > > + */ > > +int WINAPI WSASendMsg( SOCKET s, LPWSAMSG msg, DWORD dwFlags, LPDWORD lpNumberOfBytesSent, > > + LPWSAOVERLAPPED lpOverlapped, > > + LPWSAOVERLAPPED_COMPLETION_ROUTINE lpCompletionRoutine) > > This is redundant, WS2_sendto will already fetch the file descriptor, > any necessary error checking should happen there. Ok, thanks for the review. I'll try again asap. > -- > Alexandre Julliard > julliard(a)winehq.org Bruno

Sept. 30, 2013

Re: [2/3] kernel32/tests: test CopyFileEx callback and cancellation (resend)

by Nikolay Sivov

On 9/30/2013 10:10, Daniel Jeliński wrote: > 2013/9/30 Nikolay Sivov <bunglehead(a)gmail.com > <mailto:bunglehead(a)gmail.com>> > > On 9/30/2013 00:51, Daniel Jeliński wrote: > > > +struct progress_list { > + const DWORD progress_retval_init; /* value to return > from progress routine */ > + const BOOL cancel_init; /* value to set Cancel > flag to */ > + const DWORD progress_retval_end; /* value to return > from progress routine */ > + const BOOL cancel_end; /* value to set Cancel > flag to */ > + const DWORD progress_count; /* number of times > progress is invoked */ > + const BOOL copy_retval; /* expected CopyFileEx > result */ > + const DWORD lastError; /* expected CopyFileEx > error code */ > +} ; > > I don't see a point making them 'const'. > > I'm matching the formatting of existing code: > http://source.winehq.org/source/dlls/kernel32/tests/file.c#L65 > Also, what's the point of not making them const? > > +static DWORD WINAPI progress(LARGE_INTEGER TotalFileSize, > + LARGE_INTEGER TotalBytesTransferred, > + LARGE_INTEGER StreamSize, > + LARGE_INTEGER StreamBytesTransferred, > + DWORD dwStreamNumber, > + DWORD dwCallbackReason, > + HANDLE hSourceFile, > + HANDLE hDestinationFile, > + LPVOID lpData) > +{ > + progressInvoked++; > > Please pass all globals as context data with lpData, and please > use 'void*' instead of LPVOID. > > Good point about lpData. Still, does that make the patch invalid? It's not black or white, I mentioned what will be nice to do about it to make it more compact and self-contained. It doesn't mean it's invalid, it's just an obvious thing that will make it better. > Why didn't you mention that on the first review? Because sometimes I stop reading further after I see major problems. > > > Also, any comments on patch 3? > Same thing, you could easily pack everything to a single struct and pass it using this context pointer. It could also be made more compact using a single helper to compar COPYFILE2_MESSAGE value, instead of duplicating it for every message type.

Sept. 30, 2013

Re: [2/3] kernel32/tests: test CopyFileEx callback and cancellation (resend)

by Juan Lang

On Sun, Sep 29, 2013 at 11:10 PM, Daniel Jeliński <djelinski1(a)gmail.com>wrote: > 2013/9/30 Nikolay Sivov <bunglehead(a)gmail.com> > >> On 9/30/2013 00:51, Daniel Jeliński wrote: >> >> >> +struct progress_list { >>> + const DWORD progress_retval_init; /* value to return from progress >>> routine */ >>> + const BOOL cancel_init; /* value to set Cancel flag to */ >>> + const DWORD progress_retval_end; /* value to return from progress >>> routine */ >>> + const BOOL cancel_end; /* value to set Cancel flag to */ >>> + const DWORD progress_count; /* number of times progress is >>> invoked */ >>> + const BOOL copy_retval; /* expected CopyFileEx result */ >>> + const DWORD lastError; /* expected CopyFileEx error >>> code */ >>> +} ; >>> >> I don't see a point making them 'const'. >> > I'm matching the formatting of existing code: > http://source.winehq.org/source/dlls/kernel32/tests/file.c#L65 > Also, what's the point of not making them const? > It's a little strange, stylewise. More consistent with C++ style would be to make the entire struct constant. But in a test, we often eschew with such things if they distract, and here I think they do. > +static DWORD WINAPI progress(LARGE_INTEGER TotalFileSize, >>> + LARGE_INTEGER TotalBytesTransferred, >>> + LARGE_INTEGER StreamSize, >>> + LARGE_INTEGER StreamBytesTransferred, >>> + DWORD dwStreamNumber, >>> + DWORD dwCallbackReason, >>> + HANDLE hSourceFile, >>> + HANDLE hDestinationFile, >>> + LPVOID lpData) >>> +{ >>> + progressInvoked++; >>> >> Please pass all globals as context data with lpData, and please use >> 'void*' instead of LPVOID. >> > Good point about lpData. Still, does that make the patch invalid? Why > didn't you mention that on the first review? > About LPVOID - I'm matching the headers: > http://source.winehq.org/source/include/winbase.h#L910 > for the third patch: > http://source.winehq.org/source/include/winbase.h#L1018 > LPVOID is just an uglier #define of void*. It's easier to read as void*, so please use that instead. --Juan

Sept. 30, 2013

Increasing max_req_size and subsequent errors

by Hugh McMaster

I need to increase the $max_req_size in tools/make_requests. A new request structure for GetConsoleScreenBufferInfoEx is 80 bytes and primarily made up of 16 unsigned long integers. This is fine, but raises the error: wineserver: request.c:755: open_master_socket: Assertion `sizeof(union generic_reply) == sizeof(struct request_max_size)' failed. It looks like this error is connected to struct request_max_size { int pad[16]; }; in include/wine/server_protocol.h. But other than increasing the size of the array, I'm not sure how to fix this appropriately. Any advice will be appreciated.

Sept. 30, 2013

Re: atl90: Add new DLL

by Zhenbo Li

2013/9/4 Zhenbo Li <litimetal(a)gmail.com>: > This patch could fix bug 34431, which is caused by missing atl90.dll > > --- > configure | 1 + > configure.ac | 1 + > dlls/atl90/Makefile.in | 7 ++++ > dlls/atl90/atl90.c | 93 > ++++++++++++++++++++++++++++++++++++++++++++++++++ > dlls/atl90/atl90.spec | 52 ++++++++++++++++++++++++++++ > include/atlbase.h | 1 + > tools/make_specfiles | 1 + > 7 files changed, 156 insertions(+) Excuse me, this is my first time to submit a patch to Wine. And I don't know that if I had made some mistakes. Would someone please review my patch and tell me what to do? Thank you. -- Have a nice day! Zhenbo Li

Sept. 30, 2013

Re: [PATCH 1/2] setupapi: Also look in datadir/fakedlls for the re-compiled fakedlls.

by Alexandre Julliard

Huw Davies <huw(a)codeweavers.com> writes: > --- > dlls/setupapi/fakedll.c | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) I don't think that's correct. The datadir is for platform-independent files, but the fakedlls are platform specific. -- Alexandre Julliard julliard(a)winehq.org

Sept. 30, 2013

Re: [3/3] ws2_32: Implement WSASendMsg() (try 2)

by Alexandre Julliard

Bruno Jesus <00cpxxx(a)gmail.com> writes: > @@ -2427,6 +2427,46 @@ static void WINAPI WS2_GetAcceptExSockaddrs(PVOID buffer, DWORD data_size, DWORD > } > > /*********************************************************************** > + * WSASendMsg > + */ > +int WINAPI WSASendMsg( SOCKET s, LPWSAMSG msg, DWORD dwFlags, LPDWORD lpNumberOfBytesSent, > + LPWSAOVERLAPPED lpOverlapped, > + LPWSAOVERLAPPED_COMPLETION_ROUTINE lpCompletionRoutine) > +{ > + int ret = SOCKET_ERROR, fd, sock_type; > + socklen_t optlen = sizeof(sock_type); > + > + if (!msg) > + { > + SetLastError(WSAEFAULT); > + return SOCKET_ERROR; > + } > + > + if ( (fd = get_sock_fd( s, 0, NULL )) == -1) > + return SOCKET_ERROR; > + > + if (getsockopt(fd, SOL_SOCKET, SO_TYPE, &sock_type, &optlen) == -1) > + { > + SetLastError((errno == EBADF) ? WSAENOTSOCK : wsaErrno()); > + goto cleanup; > + } > + > + if (sock_type != SOCK_DGRAM && sock_type != SOCK_RAW) > + { > + SetLastError(WSAEINVAL); > + goto cleanup; > + } > + > + ret = WS2_sendto( s, msg->lpBuffers, msg->dwBufferCount, lpNumberOfBytesSent, > + dwFlags, msg->name, msg->namelen, > + lpOverlapped, lpCompletionRoutine ); This is redundant, WS2_sendto will already fetch the file descriptor, any necessary error checking should happen there. -- Alexandre Julliard julliard(a)winehq.org

Sept. 30, 2013

Re: mmdevapi/tests: test MMDevPropStore_GetAt

by Marvin

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://newtestbot.winehq.org/JobDetails.pl?Key=2403 Your paranoid android. === wvista (32 bit propstore) === propstore.c:108: Test failed: DEVPKEY_Device_FriendlyName not found

Sept. 30, 2013