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

Re: [PATCH] kernel32: Implement CompareStringOrdinal. (try 2)

by Nikolay Sivov

On 10/30/2012 01:43, Christian Costa wrote: > Try 2: > - Don't use CompareStringEx. > - Add tests > --- > dlls/kernel32/kernel32.spec | 1 + > dlls/kernel32/locale.c | 28 ++++++++++++++++++++ > dlls/kernel32/tests/locale.c | 60 +++++++++++++++++++++++++++++++++++++++++- > 3 files changed, 88 insertions(+), 1 deletion(-) > > diff --git a/dlls/kernel32/kernel32.spec b/dlls/kernel32/kernel32.spec > index b7efa0f..0bd1adc 100644 > --- a/dlls/kernel32/kernel32.spec > +++ b/dlls/kernel32/kernel32.spec > @@ -201,6 +201,7 @@ > @ stdcall CompareStringA(long long str long str long) > @ stdcall CompareStringW(long long wstr long wstr long) > @ stdcall CompareStringEx(wstr long wstr long wstr long ptr ptr long) > +@ stdcall CompareStringOrdinal(wstr long wstr long long) > @ stdcall ConnectNamedPipe(long ptr) > @ stub ConsoleMenuControl > @ stub ConsoleSubst > diff --git a/dlls/kernel32/locale.c b/dlls/kernel32/locale.c > index c41442c..07eb688 100644 > --- a/dlls/kernel32/locale.c > +++ b/dlls/kernel32/locale.c > @@ -3047,6 +3047,34 @@ INT WINAPI CompareStringA(LCID lcid, DWORD flags, > return ret; > } > > +/****************************************************************************** > + * CompareStringOrdinal (KERNEL32.@) > + */ > +INT WINAPI CompareStringOrdinal(const WCHAR *str1, INT len1, const WCHAR *str2, INT len2, BOOL ignore_case) > +{ > + int ret, len; > + > + if (!str1 || !str2) > + { > + SetLastError(ERROR_INVALID_PARAMETER); > + return 0; > + } > + if (len1 < 0) len1 = strlenW(str1); > + if (len2 < 0) len2 = strlenW(str2); > + > + len = min(len1, len2); > + if (ignore_case) > + ret = strncmpiW(str1, str2, len); > + else > + ret = strncmpW(str1, str2, len); > + > + if ((ret < 0) || (!ret && (len1 < len2))) > + return CSTR_LESS_THAN; > + else if ((ret > 0) || (!ret && (len1 > len2))) > + return CSTR_GREATER_THAN; > + return CSTR_EQUAL; > +} > + This is almost the same as your first try. You're testing a trivial case only - ASCII range. I feel like it should behave more like memcmp for case insensitive comparison, that's what documentation mildly suggests.

Oct. 30, 2012

Re: [PATCH] wininet: TLS fallback mechanism (take2)

by Alexandre Julliard

Hiroshi Miura <miurahr(a)linux.com> writes: > +#ifdef SONAME_LIBSSL > + /* can't connect if we are already connected */ > + if (connection->ssl_s) > + { > + ERR("already connected\n"); > + return ERROR_INTERNET_CANNOT_CONNECT; > + } > + > + /* connect with given TLS options */ > + res = netcon_secure_connect_setup(connection, get_tls_option()); > + if (res == ERROR_SUCCESS) > + return res; You missed some #ifdefs, this won't compile. -- Alexandre Julliard julliard(a)winehq.org

Oct. 29, 2012

Re: crypt32: Avoid double free in CRYPT_LoadSIP on error path (coverity))

by Frédéric Delanoy

On Mon, Oct 29, 2012 at 10:37 AM, Nikolay Sivov <bunglehead(a)gmail.com> wrote: > On 10/29/2012 11:33, Frédéric Delanoy wrote: >> >> FreeLibrary(temp) can be called twice on error. >> >> CID 714004 >> --- > >> error: >> FreeLibrary(lib); >> - FreeLibrary(temp); >> + if (temp != NULL) FreeLibrary(temp); >> SetLastError(TRUST_E_SUBJECT_FORM_UNKNOWN); >> return FALSE; >> } > > This is useless, FreeLibrary() checks for NULL. Right, although it uses "SetLastError( ERROR_INVALID_HANDLE)" when it encounters NULLs But the return value of FreeLibrary isn't checked, so it probably doesn't matter much. Frédéric

Oct. 29, 2012

Re: [PATCH] kernel32: Implement CompareStringOrdinal.

by Christian Costa

Le 29/10/2012 06:57, Nikolay Sivov a écrit : > On 10/29/2012 00:53, Christian Costa wrote: >> --- >> dlls/kernel32/kernel32.spec | 1 + >> dlls/kernel32/locale.c | 8 ++++++++ >> 2 files changed, 9 insertions(+) >> >> diff --git a/dlls/kernel32/kernel32.spec b/dlls/kernel32/kernel32.spec >> index b7efa0f..0bd1adc 100644 >> --- a/dlls/kernel32/kernel32.spec >> +++ b/dlls/kernel32/kernel32.spec >> @@ -201,6 +201,7 @@ >> @ stdcall CompareStringA(long long str long str long) >> @ stdcall CompareStringW(long long wstr long wstr long) >> @ stdcall CompareStringEx(wstr long wstr long wstr long ptr ptr long) >> +@ stdcall CompareStringOrdinal(wstr long wstr long long) >> @ stdcall ConnectNamedPipe(long ptr) >> @ stub ConsoleMenuControl >> @ stub ConsoleSubst >> diff --git a/dlls/kernel32/locale.c b/dlls/kernel32/locale.c >> index c41442c..83de815 100644 >> --- a/dlls/kernel32/locale.c >> +++ b/dlls/kernel32/locale.c >> @@ -3047,6 +3047,14 @@ INT WINAPI CompareStringA(LCID lcid, DWORD flags, >> return ret; >> } >> +/****************************************************************************** >> + * CompareStringOrdinal (KERNEL32.@) >> + */ >> +INT WINAPI CompareStringOrdinal(LPCWSTR str1, INT len1, LPCWSTR >> str2, INT len2, BOOL ignore_case) >> +{ >> + return CompareStringEx(NULL, ignore_case ? NORM_IGNORECASE : 0, >> str1, len1, str2, len2, NULL, NULL, 0); >> +} >> + >> /************************************************************************* >> * lstrcmp (KERNEL32.@) >> * lstrcmpA (KERNEL32.@) > This is not a way to do it, it doesn't simply work as that. See > http://msdn.microsoft.com/en-us/library/windows/desktop/dd318144%28v=vs.85%… > And it need tests of course. > > > Ok. I will see. Thanks for the link.

Oct. 29, 2012

Re: [PATCH 1/2] msvcp90: set eof state in istream<>::peek if got eof

by Alexandre Julliard

Daniel Lehman <dlehman(a)esri.com> writes: > +static void test_istream_peek(void) > +{ > + unsigned short testus, nextus; > + basic_stringstream_wchar wss; > + basic_stringstream_char ss; > + basic_string_wchar wstr; > + basic_string_char str; > + IOSB_iostate state; > + int i, next, peek; > + wchar_t wide[64]; > + > + struct _test_istream_peek { > + const char *str; > + int peek; > + int next; > + IOSB_iostate state; > + } tests[] = { > + { "", EOF, EOF, IOSTATE_eofbit }, > + { "ABCDEF", 'A', 'A', IOSTATE_goodbit }, > + }; > + > + for(i=0; i<sizeof(tests)/sizeof(tests[0]); i++) { > + /* char version */ > + call_func2(p_basic_string_char_ctor_cstr, &str, tests[i].str); > + call_func4(p_basic_stringstream_char_ctor_str, &ss, &str, OPENMODE_out|OPENMODE_in, TRUE); > + > + peek = (int)call_func1(p_basic_istream_char_peek, &ss.base.base1); > + state = (IOSB_iostate)call_func1(p_ios_base_rdstate, &ss.basic_ios.base); > + next = (int)call_func1(p_basic_istream_char_get, &ss.base.base1); 'peek' is not tested anywhere. -- Alexandre Julliard julliard(a)winehq.org

Oct. 29, 2012

Re: gdiplus: fix rounding error (?)

by Vincent Povirk

That was the worst possible defence of this patch, but I think it should be included. What it's actually doing is removing the duplicated logic between the earlier bit of GdipDrawString, where out-of-range sizes are set to 1 << 23, and this if statement where we only apply a clipping rectangle if the sizes are out of range. We could also have added a check for e.g. scaled_rect.Width >= INT_MAX here, but I think it's better that we don't duplicate the logic.

Oct. 29, 2012

Re: dplayx: Reorder some code to avoid memory leaks (coverity)

by James Eder

On Mon, Oct 29, 2012 at 2:20 AM, Rico Schüller <kgbricola(a)web.de> wrote: > On 28.10.2012 16:13, André Hentschel wrote: >> >> - lpGData->lpRemoteData = lpNewData; >> + lpGData->lpRemoteData = HeapAlloc( GetProcessHeap(), >> HEAP_ZERO_MEMORY, sizeof( dwDataSize ) ); >> + CopyMemory( lpGData->lpRemoteData, lpData, dwDataSize ); >> lpGData->dwRemoteDataSize = dwDataSize; > > >> - lpPData->lpRemoteData = lpNewData; >> + lpPData->lpRemoteData = HeapAlloc( GetProcessHeap(), >> HEAP_ZERO_MEMORY, sizeof( dwDataSize ) ); >> + CopyMemory( lpPData->lpRemoteData, lpData, dwDataSize ); >> lpPData->dwRemoteDataSize = dwDataSize; > > > Is the HEAP_ZERO_MEMORY really needed? You may kill that too while you > change those lines. > > Cheers > Rico > > I didn't look at the code too long but, sizeof( dwDataSize ) looks wrong too. -- Jim

Oct. 29, 2012

Re: [3/5] d3dx9: Introduce a function for copying pixels.

by Matteo Bruni

2012/10/29 Christian Costa <titan.costa(a)gmail.com>: > >> >> pixel_format = get_format_info(src_desc.Format); >> - if (pixel_format->type != FORMAT_ARGB) >> - { >> - FIXME("Unsupported pixel format %#x\n", src_desc.Format); >> - return E_NOTIMPL; >> - } >> + if (pixel_format->type == FORMAT_UNKNOWN) return E_NOTIMPL; >> >> > > Doesn't this still require a FIXME or at least a WARN? > There is still a FIXME in get_format_info(), that's enough I guess.

Oct. 29, 2012

Re: vbscript: Added support for negative constants

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 http://testbot.winehq.org/JobDetails.pl?Key=22550 Your paranoid android. === WNT4WSSP6 (32 bit) === No test summary line found === W2KPROSP4 (32 bit) === No test summary line found === WXPPROSP3 (32 bit) === No test summary line found === W2K3R2SESP2 (32 bit) === No test summary line found === WVISTAADM (32 bit) === No test summary line found === W2K8SE (32 bit) === No test summary line found === W7PRO (32 bit) === No test summary line found === W7PROX64 (32 bit) === No test summary line found === TEST64_W7SP1 (32 bit) === No test summary line found === W7PROX64 (64 bit) === No test summary line found === TEST64_W7SP1 (64 bit) === No test summary line found

Oct. 29, 2012

Re: [3/5] d3dx9: Introduce a function for copying pixels.

by Christian Costa

> > pixel_format = get_format_info(src_desc.Format); > - if (pixel_format->type != FORMAT_ARGB) > - { > - FIXME("Unsupported pixel format %#x\n", src_desc.Format); > - return E_NOTIMPL; > - } > + if (pixel_format->type == FORMAT_UNKNOWN) return E_NOTIMPL; > > Doesn't this still require a FIXME or at least a WARN?

Oct. 29, 2012