October 2014 - Wine-Devel - list.winehq.org

Re: [4/6] msvcrt: Added _fread_nolock_s implementation

by Iván Matellanes

On 31/10/14 07:46, Alexandre Julliard wrote: > "Iván Matellanes" <matellanesivan(a)gmail.com> writes: > >> { >> + MSVCRT_size_t ret; >> + >> + MSVCRT__lock_file(stream); >> + ret = MSVCRT__fread_nolock_s(buf, buf_size, elem_size, count, stream); >> + MSVCRT__unlock_file(stream); > > The parameter validation has to be done before locking the file. > Hi, thanks for the comment. Should all parameters be validated, or just stream != NULL? Validating the rest seems redundant to me. Regards, Iván.

Oct. 31, 2014

Re: comctl32: Avoid magic numbers

by Nikolay Sivov

On Fri, Oct 31, 2014 at 2:00 PM, Bruno Jesus <00cpxxx(a)gmail.com> wrote: > On Fri, Oct 31, 2014 at 3:11 AM, Nikolay Sivov <nsivov(a)codeweavers.com> > wrote: > >> /* FIXME: This flag is undocumented and unknown by our CompareString. > >> - * We need a define for it. > >> */ > >> - dwFlags = 0x10000000; > >> + dwFlags = LOCALE_RETURN_GENITIVE_NAMES; > >> if (!bCase) dwFlags |= NORM_IGNORECASE; > > > > > > It makes no sense to use this flag from CompareString(). I think it's > better > > to remove it along with this comment. > > Well I didn't want to touch that, my logic was that dwFlags was being > set to LOCALE_USE_CP_ACP and right after that it was set to > 0x10000000. So I looked at the other flags: > > #define LOCALE_NOUSEROVERRIDE 0x80000000 > #define LOCALE_USE_CP_ACP 0x40000000 > #define LOCALE_RETURN_NUMBER 0x20000000 > #define LOCALE_RETURN_GENITIVE_NAMES 0x10000000 > > And noticed that 0x10000000 is LOCALE_RETURN_GENITIVE_NAMES. This > patch purpose was solely to remove the magic number. Please feel free > to remove anything you wish. For the sake of history these commits are > related to it: > https://source.winehq.org/git/wine.git/commitdiff/ac323a20 > https://source.winehq.org/git/wine.git/commitdiff/64d68b10 > > Sure, I get that. My point was that such knowledge was probably gained by tracing with native comctl32, and it's not allowed. Also if Wine doesn't support such flag for CompareString() anyway no need to bother setting it, because virtual benefits of running Wine's comctl32 on Windows are insignificant. I think I'll send a patch to remove that, yeah. Thanks for bringing this issue up. > Best regards, > Bruno > > >

Oct. 31, 2014

Re: comctl32: Avoid magic numbers

by Bruno Jesus

On Fri, Oct 31, 2014 at 3:11 AM, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: >> /* FIXME: This flag is undocumented and unknown by our CompareString. >> - * We need a define for it. >> */ >> - dwFlags = 0x10000000; >> + dwFlags = LOCALE_RETURN_GENITIVE_NAMES; >> if (!bCase) dwFlags |= NORM_IGNORECASE; > > > It makes no sense to use this flag from CompareString(). I think it's better > to remove it along with this comment. Well I didn't want to touch that, my logic was that dwFlags was being set to LOCALE_USE_CP_ACP and right after that it was set to 0x10000000. So I looked at the other flags: #define LOCALE_NOUSEROVERRIDE 0x80000000 #define LOCALE_USE_CP_ACP 0x40000000 #define LOCALE_RETURN_NUMBER 0x20000000 #define LOCALE_RETURN_GENITIVE_NAMES 0x10000000 And noticed that 0x10000000 is LOCALE_RETURN_GENITIVE_NAMES. This patch purpose was solely to remove the magic number. Please feel free to remove anything you wish. For the sake of history these commits are related to it: https://source.winehq.org/git/wine.git/commitdiff/ac323a20 https://source.winehq.org/git/wine.git/commitdiff/64d68b10 Best regards, Bruno

Oct. 31, 2014

Re: [PATCH 2/2] user32/tests: Test exception handling for timer callbacks.

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://testbot.winehq.org/JobDetails.pl?Key=9924 Your paranoid android. === w7u (32 bit msg) === msg.c:12154: Test failed: unexpected hwnd 0014013C msg.c:12176: Test failed: bad time 325e7/325f6

Oct. 31, 2014

Re: [4/6] msvcrt: Added _fread_nolock_s implementation

by Alexandre Julliard

"Iván Matellanes" <matellanesivan(a)gmail.com> writes: > { > + MSVCRT_size_t ret; > + > + MSVCRT__lock_file(stream); > + ret = MSVCRT__fread_nolock_s(buf, buf_size, elem_size, count, stream); > + MSVCRT__unlock_file(stream); The parameter validation has to be done before locking the file. -- Alexandre Julliard julliard(a)winehq.org

Oct. 31, 2014

Re: comctl32: Avoid magic numbers

by Nikolay Sivov

> /* FIXME: This flag is undocumented and unknown by our CompareString. > - * We need a define for it. > */ > - dwFlags = 0x10000000; > + dwFlags = LOCALE_RETURN_GENITIVE_NAMES; > if (!bCase) dwFlags |= NORM_IGNORECASE; It makes no sense to use this flag from CompareString(). I think it's better to remove it along with this comment.

Oct. 31, 2014

Re: [2/2] oleaut32: Implement TLB dependencies lookup in resources (resend 3 - last submit before abandon)

by Alexandre Julliard

Guillaume Charifi <guillaume.charifi(a)sfr.fr> writes: > + strcpyW(szTLBPath, szPath); > + snprintfW(szStringName, sizeof(szStringName)/sizeof(WCHAR), resFormatStrW, PtrToInt(lpszIntName)); > + strcatW(szTLBPath, szStringName); > + > + ret = TLB_PEFile_Open(szPath, PtrToInt(lpszIntName), &pBase, &dwTLBLength, &pFile); > + if (ret == TYPE_E_CANTLOADLIBRARY) > + ret = TLB_NEFile_Open(szPath, PtrToInt(lpszIntName), &pBase, &dwTLBLength, &pFile); > + if (ret == TYPE_E_CANTLOADLIBRARY) > + ret = TLB_Mapping_Open(szPath, &pBase, &dwTLBLength, &pFile); > + if (SUCCEEDED(ret)) > + { > + if (dwTLBLength >= 4) > + { > + DWORD dwSignature = FromLEDWord(*((DWORD*) pBase)); > + if (dwSignature == MSFT_SIGNATURE) > + pTLib = ITypeLib2_Constructor_MSFT(pBase, dwTLBLength); > + else if (dwSignature == SLTG_SIGNATURE) > + pTLib = ITypeLib2_Constructor_SLTG(pBase, dwTLBLength); > + else > + { > + FIXME("Header type magic 0x%08x not supported.\n", dwSignature); > + ret = E_FAIL; > + } > + } > + else > + ret = E_FAIL; > + IUnknown_Release(pFile); > + } You should avoid duplicating all that code. > + if(pTLib) { > + ITypeLibImpl *impl = impl_from_ITypeLib2(pTLib); > + ret = UuidEqual((UUID *)This->guid, (UUID *)TLB_get_guid_null(impl->guid), &rpc_stat); > + ITypeLib_Release((ITypeLib *)pTLib); > + > + ret = (ret == TRUE) ? LoadTypeLibEx(szTLBPath, REGKIND_NONE, (ITypeLib **)&pTLib) : E_FAIL; > + } That's pretty ugly. -- Alexandre Julliard julliard(a)winehq.org

Oct. 31, 2014

Re: [PATCH 4/5] d3d9/tests: Add a test for ATI1N and ATI2N texture formats.

by Henri Verbeet

On 30 October 2014 14:13, Matteo Bruni <matteo.mystral(a)gmail.com> wrote: > Initially it was but then left it there on purpose, to be able to > check whether the tested driver takes the AMD or the Nvidia approach. > Not sure it is worth it though. I think it's mostly just adding noise when running the tests manually on Windows.

Oct. 30, 2014

Re: Need feedback on first four UTF-7 patches

by Alexandre Julliard

Alex Henrie <alexhenrie24(a)gmail.com> writes: > 2014-10-29 14:37 GMT-06:00 Sebastian Lackner <sebastian(a)fds-team.de>: >> For the tests: The main issue (bad test coverage) is fixed at my >> opinion, but there are still lot of small things which could be >> improved to make the code more readable. Most of it was already told >> on IRC, but I will go over patch 1 one more time and tell you again. > > Again, thanks for the feedback. > > For the record, I am holding off on the following suggestions until I > receive feedback from Alexandre: If you want more feedback, you should first address all the issues that you have already received feedback about. -- Alexandre Julliard julliard(a)winehq.org

Oct. 30, 2014

Re: msi: Format registry values before interpreting value type.

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://testbot.winehq.org/JobDetails.pl?Key=9898 Your paranoid android. === wxppro (32 bit action) === The test timed out === w7u (32 bit action) === The test timed out

Oct. 30, 2014