August 2008 - Wine-Devel - list.winehq.org

Re: crypt32(4/7): Add tests for CMSG_CMS_SIGNER_INFO_PARAM
by Eric Pouech Aug. 20, 2008

Aug. 20, 2008

Juan Lang a écrit : > --Juan > ------------------------------------------------------------------------ > > From 7eaf8a741cc4ff2035491f36ba9b8f7c5d650458 Mon Sep 17 00:00:00 2001 > From: Juan Lang <juan.lang(a)gmail.com> > Date: Mon, 18 Aug 2008 13:09:24 -0700 > Subject: [PATCH] Add tests for CMSG_CMS_SIGNER_INFO_PARAM > > --- > dlls/crypt32/tests/msg.c | 135 ++++++++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 135 insertions(+), 0 deletions(-) > > diff --git a/dlls/crypt32/tests/msg.c b/dlls/crypt32/tests/msg.c > index 86ce6b1..ce4affd 100644 > --- a/dlls/crypt32/tests/msg.c > +++ b/dlls/crypt32/tests/msg.c > @@ -2265,9 +2265,56 @@ static void compare_signer_info(const CMSG_SIGNER_INFO *got, > /* FIXME: check more things */ > } > > +static void compare_cms_signer_info(const CMSG_CMS_SIGNER_INFO *got, > + const CMSG_CMS_SIGNER_INFO *expected) > +{ > + ok(got->dwVersion == expected->dwVersion, "Expected version %d, got %d\n", > + expected->dwVersion, got->dwVersion); > + ok(got->SignerId.dwIdChoice == expected->SignerId.dwIdChoice, > + "Expected id choice %d, got %d\n", expected->SignerId.dwIdChoice, > + got->SignerId.dwIdChoice); > + if (got->SignerId.dwIdChoice == expected->SignerId.dwIdChoice) > + { > + if (got->SignerId.dwIdChoice == CERT_ID_ISSUER_SERIAL_NUMBER) > + { > + ok(got->SignerId.IssuerSerialNumber.Issuer.cbData == > + expected->SignerId.IssuerSerialNumber.Issuer.cbData, > + "Expected issuer size %d, got %d\n", > + expected->SignerId.IssuerSerialNumber.Issuer.cbData, > + got->SignerId.IssuerSerialNumber.Issuer.cbData); > + ok(!memcmp(got->SignerId.IssuerSerialNumber.Issuer.pbData, > + got->SignerId.IssuerSerialNumber.Issuer.pbData, shouldn't it be expected->SignerId.IssuerSerialNumber.Issuer.pbData, > + got->SignerId.IssuerSerialNumber.Issuer.cbData), > + "Unexpected issuer\n"); > + ok(got->SignerId.IssuerSerialNumber.SerialNumber.cbData == > + expected->SignerId.IssuerSerialNumber.SerialNumber.cbData, > + "Expected serial number size %d, got %d\n", > + expected->SignerId.IssuerSerialNumber.SerialNumber.cbData, > + got->SignerId.IssuerSerialNumber.SerialNumber.cbData); > + ok(!memcmp(got->SignerId.IssuerSerialNumber.SerialNumber.pbData, > + got->SignerId.IssuerSerialNumber.SerialNumber.pbData, ditto ? > + got->SignerId.IssuerSerialNumber.SerialNumber.cbData), > + "Unexpected serial number\n"); > + } > + else > + { > + ok(got->SignerId.KeyId.cbData == expected->SignerId.KeyId.cbData, > + "expected key id size %d, got %d\n", > + expected->SignerId.KeyId.cbData, got->SignerId.KeyId.cbData); > + ok(!memcmp(expected->SignerId.KeyId.pbData, > + got->SignerId.KeyId.pbData, got->SignerId.KeyId.cbData), > + "unexpected key id\n"); > + } > + } > + /* FIXME: check more things */ > +} > + > static const BYTE signedWithCertAndCrlComputedHash[] = { > 0x08,0xd6,0xc0,0x5a,0x21,0x51,0x2a,0x79,0xa1,0xdf,0xeb,0x9d,0x2a,0x8f,0x26, > 0x2f }; > +static BYTE keyIdIssuer[] = { > +0x30,0x13,0x31,0x11,0x30,0x0f,0x06,0x0a,0x2b,0x06,0x01,0x04,0x01,0x82,0x37, > +0x0a,0x07,0x01,0x04,0x01,0x01 }; > > static void test_decode_msg_get_param(void) > { > @@ -2354,6 +2401,32 @@ static void test_decode_msg_get_param(void) > compare_signer_info((CMSG_SIGNER_INFO *)buf, &signer); > CryptMemFree(buf); > } > + /* Getting the CMS signer info of a PKCS7 message is possible. */ > + size = 0; > + ret = CryptMsgGetParam(msg, CMSG_CMS_SIGNER_INFO_PARAM, 0, NULL, &size); > + todo_wine > + ok(ret, "CryptMsgGetParam failed: %08x\n", GetLastError()); > + if (ret) > + buf = CryptMemAlloc(size); > + else > + buf = NULL; > + if (buf) > + { > + CMSG_CMS_SIGNER_INFO signer = { 0 }; > + > + signer.dwVersion = 1; > + signer.SignerId.dwIdChoice = CERT_ID_ISSUER_SERIAL_NUMBER; > + signer.SignerId.IssuerSerialNumber.Issuer.cbData = > + sizeof(encodedCommonName); > + signer.SignerId.IssuerSerialNumber.Issuer.pbData = encodedCommonName; > + signer.SignerId.IssuerSerialNumber.SerialNumber.cbData = > + sizeof(serialNum); > + signer.SignerId.IssuerSerialNumber.SerialNumber.pbData = serialNum; > + signer.HashAlgorithm.pszObjId = oid_rsa_md5; > + CryptMsgGetParam(msg, CMSG_CMS_SIGNER_INFO_PARAM, 0, buf, &size); > + compare_cms_signer_info((CMSG_CMS_SIGNER_INFO *)buf, &signer); > + CryptMemFree(buf); > + } > /* index is ignored when getting signer count */ > size = sizeof(value); > ret = CryptMsgGetParam(msg, CMSG_SIGNER_COUNT_PARAM, 1, &value, &size); > @@ -2383,6 +2456,68 @@ static void test_decode_msg_get_param(void) > CMSG_COMPUTED_HASH_PARAM, signedWithCertAndCrlComputedHash, > sizeof(signedWithCertAndCrlComputedHash)); > CryptMsgClose(msg); > + > + msg = CryptMsgOpenToDecode(PKCS_7_ASN_ENCODING, 0, 0, 0, NULL, NULL); > + ret = CryptMsgUpdate(msg, signedKeyIdEmptyContent, > + sizeof(signedKeyIdEmptyContent), TRUE); > + ok(ret, "CryptMsgUpdate failed: %08x\n", GetLastError()); > + size = sizeof(value); > + ret = CryptMsgGetParam(msg, CMSG_SIGNER_COUNT_PARAM, 0, &value, &size); > + ok(ret, "CryptMsgGetParam failed: %08x\n", GetLastError()); > + ok(value == 1, "Expected 1 signer, got %d\n", value); > + /* Getting the regular (non-CMS) signer info from a CMS message is also > + * possible.. > + */ > + size = 0; > + ret = CryptMsgGetParam(msg, CMSG_SIGNER_INFO_PARAM, 0, NULL, &size); > + todo_wine > + ok(ret, "CryptMsgGetParam failed: %08x\n", GetLastError()); > + if (ret) > + buf = CryptMemAlloc(size); > + else > + buf = NULL; > + if (buf) > + { > + CMSG_SIGNER_INFO signer; > + BYTE zero = 0; > + > + /* and here's the little oddity: for a CMS message using the key id > + * variant of a SignerId, retrieving the CMSG_SIGNER_INFO param yields > + * a signer with a zero (not empty) serial number, and whose issuer is > + * an RDN with OID szOID_KEYID_RDN, value type CERT_RDN_OCTET_STRING, > + * and value of the key id. > + */ > + signer.dwVersion = CMSG_SIGNED_DATA_V3; > + signer.Issuer.cbData = sizeof(keyIdIssuer); > + signer.Issuer.pbData = keyIdIssuer; > + signer.SerialNumber.cbData = 1; > + signer.SerialNumber.pbData = &zero; > + CryptMsgGetParam(msg, CMSG_SIGNER_INFO_PARAM, 0, buf, &size); > + compare_signer_info((CMSG_SIGNER_INFO *)buf, &signer); > + CryptMemFree(buf); > + } > + size = 0; > + ret = CryptMsgGetParam(msg, CMSG_CMS_SIGNER_INFO_PARAM, 0, NULL, &size); > + todo_wine > + ok(ret, "CryptMsgGetParam failed: %08x\n", GetLastError()); > + if (ret) > + buf = CryptMemAlloc(size); > + else > + buf = NULL; > + if (buf) > + { > + CMSG_CMS_SIGNER_INFO signer = { 0 }; > + > + signer.dwVersion = CMSG_SIGNED_DATA_V3; > + signer.SignerId.dwIdChoice = CERT_ID_KEY_IDENTIFIER; > + signer.SignerId.KeyId.cbData = sizeof(serialNum); > + signer.SignerId.KeyId.pbData = (BYTE *)serialNum; > + signer.HashAlgorithm.pszObjId = oid_rsa_md5; > + CryptMsgGetParam(msg, CMSG_CMS_SIGNER_INFO_PARAM, 0, buf, &size); > + compare_cms_signer_info((CMSG_CMS_SIGNER_INFO *)buf, &signer); > + CryptMemFree(buf); > + } > + CryptMsgClose(msg); > } > > static void test_decode_msg(void) > -- 1.5.2.2 > ------------------------------------------------------------------------ > > -- 1.5.2.2 ------------------------------------------------------------------------ -- Eric Pouech "The problem with designing something completely foolproof is to underestimate the ingenuity of a complete idiot." (Douglas Adams)

2 1

Re: [PATCH] AcceptEx Impelementation
by Kai Blin Aug. 20, 2008

Aug. 20, 2008

Hi Scott, I didn't see a reply to your AcceptEx patches yet, sorry if I missed it and you get this again. Please submit one patch per email only. I admittedly don't know much about the wineserver stuff, but I've noticed a couple of style issues in the winsock code: The indentation common in socket.c is either a two-space indent or a four-space indent. Please don't add new code with an eight-space indent. Also, the common convention for ifs is if (cond) { and not if(cond){ Apart from those minor nitpicks, the code looks good. Cheers, Kai -- Kai Blin WorldForge developer http://www.worldforge.org/ Wine developer http://wiki.winehq.org/KaiBlin Samba team member http://www.samba.org/samba/team/ -- Will code for cotton.

5 9

Re: [9/10] WineD3D: Update the overlay when the destination was drawn to
by Alexandre Julliard Aug. 20, 2008

Aug. 20, 2008

Stefan Dösinger <stefan(a)codeweavers.com> writes: > @@ -4550,13 +4558,22 @@ static WINED3DSURFTYPE WINAPI IWineD3DSurfaceImpl_GetImplType(IWineD3DSurface *i > static HRESULT WINAPI IWineD3DSurfaceImpl_DrawOverlay(IWineD3DSurface *iface) { > IWineD3DSurfaceImpl *This = (IWineD3DSurfaceImpl *) iface; > HRESULT hr; > + static BOOL recursive = FALSE; That static looks very suspicious. It should either be stored in the object or use some sort of locking. -- Alexandre Julliard julliard(a)winehq.org

2 1

Re: 1/3 dpnet: Add basic DirectPlay8Peer implementation
by Alexandre Julliard Aug. 20, 2008

Aug. 20, 2008

Alexander Nicolaysen Sørnes <alex(a)thehandofagony.com> writes: > +static HRESULT WINAPI IDirectPlay8PeerImpl_Initialize(PDIRECTPLAY8PEER iface, PVOID CONST pvUserContext, CONST PFNDPNMESSAGEHANDLER pfn, CONST DWORD dwFlags) > +{ > + FIXME("(%p)->(%p,%p,%x): stub\n", iface, pvUserContext, pfn, dwFlags); > + > + return DPN_OK; > +} > + > +static HRESULT WINAPI IDirectPlay8PeerImpl_EnumServiceProviders(PDIRECTPLAY8PEER iface, CONST GUID *CONST pguidServiceProvider, CONST GUID *CONST pguidApplication, DPN_SERVICE_PROVIDER_INFO *CONST pSPInfoBuffer, DWORD *CONST pcbEnumData, DWORD *CONST pcReturned, CONST DWORD dwFlags) > +{ > + FIXME("(%p)->(%p,%p,%p,%p,%p,%x): stub\n", iface, pguidServiceProvider, pguidApplication, pSPInfoBuffer, pcbEnumData, pcReturned, dwFlags); > + return DPN_OK; > +} Returning OK from stubs is usually not a good idea. > +static const IDirectPlay8PeerVtbl DirectPlay8Peer_Vtbl = > +{ > + IDirectPlay8PeerImpl_QueryInterface, > + IDirectPlay8PeerImpl_AddRef, > + IDirectPlay8PeerImpl_Release, > + IDirectPlay8PeerImpl_Initialize, > + IDirectPlay8PeerImpl_EnumServiceProviders > +}; You can't initialize a vtbl partially, you need to stub all the functions. -- Alexandre Julliard julliard(a)winehq.org

2 1

Re: gdi32/tests/bitmap: Fix win9x detection
by Alexandre Julliard Aug. 20, 2008

Aug. 20, 2008

Michael Karcher <wine(a)mkarcher.dialup.fu-berlin.de> writes: > In commit 48bb687dfb00b00f789b688459ad5d0ec77a617b, win9x detection was > changed to use the desktop window instead of a private EDIT window as > target for GetWindowLongW. GetWindowLongW(GetDesktopWindow(),GWLP_WNDPROC) > fails on wine (and probably on WinNT as well, as the counts on > test.winehq.org indicate). > > I am unsure about this patch, as I don't know whether is_win9x now > gets true if one has a Windows 98 system with the Microsoft Layer > for Unicode installed. The right way is to get rid of the check, especially since the GetWindowLongW behavior is irrelevant to what the test is doing. It should be using something like broken() instead. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: msxml3: Add ISAXContentHandler_endPrefix event (1/9) [try2]
by Alexandre Julliard Aug. 20, 2008

Aug. 20, 2008

Piotr Caban <piotr.caban(a)gmail.com> writes: > @@ -117,6 +120,29 @@ static inline saxattributes *impl_from_ISAXAttributes( ISAXAttributes *iface ) > } > > > +static HRESULT namespacePush(saxlocator *locator, int ns) > +{ > + if(locator->nsStackLast>=locator->nsStackSize) > + { > + int *new_stack; > + > + locator->nsStackSize *= 2; > + new_stack = HeapReAlloc(GetProcessHeap(), 0, > + locator->nsStack, locator->nsStackSize); > + if(!new_stack) return E_OUTOFMEMORY; > + locator->nsStack = new_stack; You need to preserve nsStackSize on error too. -- Alexandre Julliard julliard(a)winehq.org

1 0

Avoiding charset corruption when sending patches
by Dan Kegel Aug. 20, 2008

Aug. 20, 2008

Short story: Patches that touch non-Latin1 .rc files might need to be sent as attached non-text files rather than inline or as .txt files, to avoid charset corruption. Some examples: http://winehq.org/pipermail/wine-patches/2008-August/059095.html might suffer corruption (I can't tell offhand). http://winehq.org/pipermail/wine-patches/2008-August/059405.html looks safe, though. Long story: Patchwatcher was converting patches to utf-8, causing the patch [1/3] wined3d: Move creating a texture id from PreLoad to BindTexture to not apply. Thanks to Henri Verbeet for diagnosing the problem. The patchwatcher web page may need some surgery to deal with this. Currently it uses a single .txt file for the email message and the patch, which probably tempts the web server and browser to do charset conversions. I will probably switch the suffix to .bin to discourage this. Even longer story: So, what character set is the Wine source tree encoded with? It looks like a mixture: 1. in general, .c and .h files are encoded in Latin-1 aka iso8859-1. 2. include/*.h are encoded in ASCII and are not allowed to have non-ascii chars; see http://bugs.winehq.org/show_bug.cgi?id=5420 3. *_XX.rc (or XX.rc) are in the character set that Windows defaults to for language XX, unless changed by a #pragma code_page... in the .rc file. http://wiki.winehq.org/Developers-Hints#head-ef42a958e4f633dbd4a0ae42649fa0… Which brings us to the question: since email clients love changing charsets of text messages, how can we avoid corrupting patches when sending them via email? It kind of looks like patches to .rc files must be attached rather than inline, and should be attached as .patch, .diff, or .bin files so they are marked with the mime type application/octet-stream which should inhibit all charset conversions. (Attaching as .txt might tempt mail clients to perform charset conversions on them.) - Dan

2 1

Re: [PATCH 2/5] mstask: Implemented (Set|Get)ApplicationName. (Revision)
by Alexandre Julliard Aug. 20, 2008

Aug. 20, 2008

Roy Shea <royshea(a)gmail.com> writes: > @@ -323,16 +324,62 @@ static HRESULT WINAPI MSTASK_ITask_SetApplicationName( > ITask* iface, > LPCWSTR pwszApplicationName) > { > - FIXME("(%p, %s): stub\n", iface, debugstr_w(pwszApplicationName)); > - return E_NOTIMPL; > + DWORD n; > + TaskImpl *This = (TaskImpl *)iface; > + > + TRACE("(%p, %s)\n", iface, debugstr_w(pwszApplicationName)); > + > + HeapFree(GetProcessHeap(), 0, This->applicationName); > + This->applicationName = NULL; You need to preserve the previous value on error. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [NJS#80611] re: msvcmaker: fix project file generation under MinGW32
by Hongbo Ni Aug. 20, 2008

Aug. 20, 2008

At 03:25 AM 2008-08-20, Dan Kegel wrote: >Hongbo, it looks like your patch failed to apply because you did not >preserve whitespace exactly. Your patch uses all spaces >for the context, but the file contains tabs, so it does not match, >and patch fails. Thank you Dan, and thanks to Patchwatcher. I will fix it and resubmit. Hongbo Ni Hongbo Ni <hongbo(a)njstar.com> +----------------------------------------------------------------+ * NJStar Software Corp., P.O. Box 40, Epping NSW 2121, Australia. * Tel: +61-2-9869-1088 WebSite: http://www.njstar.com * Fax: +61-2-9869-1099 Email: http://njstar.com/contact/ * eFax: +1-253-595-4788 +----------------------------------------------------------------+

1 0

Incidental patch cleaning?
by Andrew Talbot Aug. 19, 2008

Aug. 19, 2008

Hi, Is it possible to sneak in a bit of patch cleaning within the new software? It would be useful to incidentally remove any trailing white space that happens to exist within the scope of a patch. -- Andy.

3 2

RE: dinput: Add mouse warp override config option (resend)
by Stefan Dösinger Aug. 19, 2008

Aug. 19, 2008

Just for the record, I think we really want this patch. XInput 2 is currently under development, so we cannot fix the issue properly right now. Even if we have XI2, there will still be old X servers around for ages. > -----Original Message----- > From: wine-patches-bounces(a)winehq.org [mailto:wine-patches- > bounces(a)winehq.org] On Behalf Of Vitaliy Margolen > Sent: Monday, August 18, 2008 8:18 PM > To: wine-patches(a)winehq.org > Subject: dinput: Add mouse warp override config option (resend) > > > Set [HKCU\Software\Wine\DirectInput] (or application specific key) > "MouseWarpOverride" to: > "force" - always warp > "disable" - newer warp > "anything else" - warp when needed - current behavior (default) > --- > dlls/dinput/mouse.c | 39 ++++++++++++++++++++++++++++++++++----- > 1 files changed, 34 insertions(+), 5 deletions(-)

3 5

Translating kernel32/nls/winerr_eng
by Alexander Nicolaysen Sørnes Aug. 19, 2008

Aug. 19, 2008

Hello, I'm translating the winerr messages to Norwegian, but I'm not completely sure about the LanguageNames line. Should the code be the same as the language ID (0x414)? LanguageNames=(NOR=0x414:winerr) This seems to be the case for English, but not French & German. Alexander N. Sørnes

1 0

Re: winmm: Test opening the same device twice
by Eric Pouech Aug. 19, 2008

Aug. 19, 2008

Maarten Lankhorst a écrit : > Behavioral test to show that wine winmm needs fixing. > --- > ------------------------------------------------------------------------ > > this likely is going to fail on old boxes there's no requirement on MS side that a PCM renderer has to be opened twice so having it as a test is wrong some old cards with OSS as a driver forbid the card to be opened twice (especially in different processes). IIRC, there was a bit of support in the OSS driver (not sure about the ALSA one) to allow > 1 opening in the same process (and also opening one handle for sound capture and another one for sound rendering). but this didn't provide support for mixing the output from the two different handles, which is likely what part of what you're trying to do assumes (especially for dsound plus standard winmm output) the only way out is to have the Linux (or Unix) driver handle several opening including the mixing of the two streams if the driver doesn't provide this flexibility, I don't see a point of implementing it in Wine A+ -- Eric Pouech "The problem with designing something completely foolproof is to underestimate the ingenuity of a complete idiot." (Douglas Adams)

2 1

patchwatcher and changes to configure.h.in [was: re: ntdll/file: Add sub-second resolution to NtQueryInformationFile]
by Dan Kegel Aug. 19, 2008

Aug. 19, 2008

>> Btw, how does patchwatcher handle that? >> Does it detect that in the case of changing config.h.in, >> dlls/Makefile.in, server/protocol.def it needs >> to regenerate the files by running autoconf, >> tools/make_requests, or tools/make_makefiles? > > Not at all. Patches gratefully accepted. Status: - posted note about two bugs in patch series handling, will fix tomorrow - "time baseline_tests" doesn't work (limitation of time cmd), need to fix borkage there. Sloppy. - Austin sent in a patch to install gecko - I'm close to having valgrind integration, but haven't looked at how to make it fast enough yet. - user32:monitor.c:262: Test failed: Failed to reset default resolution is happening lately; should probably restart X server between runs

2 2

re: msvcmaker: fix project file generation under MinGW32
by Dan Kegel Aug. 19, 2008

Aug. 19, 2008

Hi Hongbo, it looks like your patch failed to apply because you did not preserve whitespace exactly. Your patch uses all spaces for the context, but the file contains tabs, so it does not match, and patch fails. - Dan ---------- Forwarded message ---------- From: <patchwatcher(a)kegel.com> Date: Tue, Aug 19, 2008 at 10:16 AM Subject: Patchwatcher: failed to apply: msvcmaker: fix project file generation under MinGW32 To: dank(a)kegel.com Hi! This is the experimental automated wine patchwatcher thingy. The latest git sources were built and tested with your patch "msvcmaker: fix project file generation under MinGW32" Result: the patch failed to apply. You can retrieve the full build results at http://kegel.com/wine/patchwatcher/results/3.log and see the patch as parsed at http://kegel.com/wine/patchwatcher/results/3.txt See http://kegel.com/wine/patchwatcher/results for more info.

1 0

Patchwatcher status
by Dan Kegel Aug. 19, 2008

Aug. 19, 2008

Patchwatcher falsely complained that [2/17] richedit: Removed assumption about the order of rtf indent didn't apply because the regexp I used to detect the end of a patch series falsely matched the first patch in a series of 1x patches. Here's the fix: --- patchwatcher.sh (revision 150) +++ patchwatcher.sh (working copy) @@ -283,7 +283,7 @@ cat $PATCHES/$NEXT.log fi # Use a regexp with a back reference to detect last patch in a series and break out - if egrep -q 'Subject:.*[0-9]+/[0-9]+' $PATCHES/$NEXT.txt && ! egrep -q 'Subject:.*([0-9]+)/\1' $PATCHES/$NEXT.txt + if egrep -q 'Subject:.*[0-9]+/[0-9]+' $PATCHES/$NEXT.txt && ! egrep -q 'Subject:.*([0-9]+)/\1[^0-9]' $PATCHES/$NEXT.txt then echo In middle of patch series, not wiping tree NEXT=`expr $NEXT + 1` And it falsely complained that various of adam(a)tpetaccia.com's patches failed because it didn't handle the case of multiple patch series sent by the same author back to back and mixed together by email delays. Here's a fix: --- get-patches.pl (revision 150) +++ get-patches.pl (working copy) @@ -147,7 +147,7 @@ $series_num_patches = $num_patches; } - if ($series_sender ne $sender) { + if ($series_sender ne $sender || $series_num_patches != $num_patches) { #print "Ignoring series for now, will try later; sender $sender, num_patches $num_patches, subject ".$header->get('Subject')."\n"; # can't handle multiple series at once just yet, let it sit return; That still won't handle the case of two patch series of the same length being sent back to back by the same author, but that's ambiguous enough that even humans might be confused, so it should tide us over until I rewrite the patch series detector to handle incomplete series better. I'm on vacation so I can't apply these patches to the running instance, I'll do it on tuesday. - Dan

2 3

New kernel development guide might be good model for Wine...
by Dan Kegel Aug. 19, 2008

Aug. 19, 2008

This guide seems rather well written: http://ldn.linuxfoundation.org/how-participate-linux-community Wine could probably use something similar.

6 10

Re: Raising exception for SC_RPC_HANDLE being NULL
by Rob Shearman Aug. 19, 2008

Aug. 19, 2008

2008/8/18 Michael Martin <martinmnet(a)hotmail.com>: > > In widl code generated for services an > exception is raised for null string with RPC_X_NULL_REF_POINTER > In windows calls made to for example CreateService with all NULL's > checks are made on the handle being null and its checked first before null strings. > > Windows documentation on RPC_X_ exceptions say that > RPC_X_SS_IN_NULL_CONTEXT is raised for null handles. > > suggested patch > > Index: widl/client.c > =================================================================== > --- widl/client.c (trunk) > +++ widl/client.c (working copy) > @@ -67,6 +67,15 @@ > indent--; > print_client("}\n\n"); > } > + else if ((var->type->name) && (strncmp(var->type->name,"SC_RPC_HANDLE", 13)==0)) How on earth do you think it is the right fix to hard-code a services type into widl? Did you not notice that SC_RPC_LOCK also needs the right treatment and that renaming SC_RPC_HANDLE to XXXFOOBAR shouldn't make any difference to the generated code? The correct fix is to check for context handles and the output the below code. > + { > + print_client("if (!%s)\n",var->name); > + print_client("{\n"); > + indent++; > + print_client("RpcRaiseException(RPC_X_SS_IN_NULL_CONTEXT);\n"); > + indent--; > + print_client("}\n\n"); > + } > } > } -- Rob Shearman

1 0

Re: ole32 [2/2] (msi [9.5/13]): Storage sharing should be implemented inole32, not the lower-level file API
by Dmitry Timoshkov Aug. 19, 2008

Aug. 19, 2008

"James Hawkins" <jhawkins(a)codeweavers.com> wrote: > - { FALSE, ERROR_SHARING_VIOLATION, FALSE }, > - { TRUE, ERROR_SUCCESS, TRUE } > + { TRUE, ERROR_SUCCESS }, > + { TRUE, ERROR_SUCCESS }, > + { TRUE, ERROR_SUCCESS }, > + { TRUE, ERROR_SUCCESS }, > + { FALSE, ERROR_SHARING_VIOLATION }, > + { FALSE, ERROR_SHARING_VIOLATION }, > + { FALSE, ERROR_SHARING_VIOLATION }, > + { TRUE, ERROR_SUCCESS }, > + { FALSE, ERROR_SHARING_VIOLATION }, > + { FALSE, ERROR_SHARING_VIOLATION }, > + { FALSE, ERROR_SHARING_VIOLATION }, > + { TRUE, ERROR_SUCCESS }, > + { FALSE, ERROR_SHARING_VIOLATION }, > + { FALSE, ERROR_SHARING_VIOLATION }, > + { FALSE, ERROR_SHARING_VIOLATION }, > + { TRUE, ERROR_SUCCESS } > }; You forgot to remove 'BOOL todo;' from struct access_res definition. Also making the arrays of structures 'static const' while you are at it won't hurt (same for other similar tests). -- Dmitry.

1 0

Re: kernel32: Test the file access and share modes of CreateFile
by Dmitry Timoshkov Aug. 19, 2008

Aug. 19, 2008

"James Hawkins" <jhawkins(a)codeweavers.com> wrote: > Changelog: > * Test the file access and share modes of CreateFile. Of course, the more tests - the better, but isn't that a duplicate of existing test_file_sharing()? Besides win9x will require fixing it all over the place... -- Dmitry.

2 1

[TRY4]user32/tests: add tests for creating subclassed builtin-class windows
by Hongbo Ni Aug. 19, 2008

Aug. 19, 2008

Hello All, I am adding tests to test the unicodeness of builtin class window created after the class is globally subclassed. http://www.winehq.org/pipermail/wine-patches/2008-August/059354.html It tests the following builtin classes: Edit, Button, Static, ComboBox, ComboLBox, ListBox, #32770 (dialog), ScrollBar, Message, #32768 (menu), #32769 (desktop), #32772 (icon title), DDEMLEvent, #32771 (task switch) For classes that have text, wine behave different when the class is subclassed using a Unicode procudure. Todo_wine has been added. Thanks to Dmitry Timoshkov, I also added some tests before the builin class is subclassed. It turns out CreateWindowExW can not create window for class DDEMLEvent and #32771. Marked with todo_wine. The test passed for me on Windows XP, Vista and one Wine. Dmitry Timoshkov also confirmed pass on Windows XP and Vista. How can I improve those tests ? Hongbo Ni _________________________________________________________________ It's simple! Sell your car for just $40 at CarPoint.com.au http://a.ninemsn.com.au/b.aspx?URL=http%3A%2F%2Fsecure%2Dau%2Eimrworldwide%…

1 0

Re: [TRY3]user32/tests: add tests for creating subclassedbuiltin-class windows
by Dmitry Timoshkov Aug. 19, 2008

Aug. 19, 2008

"Hongbo Ni" <hongbo(a)njstar.com> wrote: > Changes since TRY2 - thanks to Dmitry Timoshkov for advice. > > 1. adds extra bulitin classes: ScrollBar,Message,DDEMLEvent & all #327xx; > 2. make sure created hwnd is valid; > 3. more tests for unicodeness of built-in-class window before subclassing; > 4. add more todo_wine for those extra classes without subclassing. > > Tested on Windows Vista SP1, XP SP3. > +#include "windows.h" Please avoid windows.h (include everything) inclusion, find an appropriate set of smaller include files instead, winnls.h in this case. The test looks good to me. it passes for me under XP SP3 (32 bit), and Vista SP1 x64 (native 64-bit and emulated 32-bit modes), although in order to pass in 64-bit mode I had to apply the following patch -#define IS_WNDPROC_HANDLE(x) (((ULONG_PTR)(x) >> 16) == (~((ULONG_PTR)0) >> 16)) +#define IS_WNDPROC_HANDLE(x) (((ULONG_PTR)(x) >> 16) == ((~(ULONG)0) >> 16)) but that's not your fault. -- Dmitry.

2 1

Re: shlwapi: add some more tests for UrlUnEscape
by Michael Karcher Aug. 18, 2008

Aug. 18, 2008

Am Montag, den 18.08.2008, 08:46 +0000 schrieb Louis. Lenders: > hi, tests show that UrlUnEscape still converts the url-string if the > bufferpointer is set to NULL, and the > URL_UNESCAPE_INPLACE is set. Test passes fine with native shlwapi, and > in wine after applying the previous sent patch : > http://www.winehq.org/pipermail/wine-patches/attachments/20080814/d5feb31b/… Please resend the patch including the testcase, instead of sending a testcase alone. This puts test & fix into the same entry in git history, so that a later review of the changes is eased by having the appropriate testcase directly at hand. Patches including testcases also have a higher chance of getting committed. Regards, Michael Karcher

1 1

Re: Add TRACEs to NtCreateFile returns
by Michael Karcher Aug. 18, 2008

Aug. 18, 2008

Another review. Am Sonntag, den 20.07.2008, 22:42 +0200 schrieb Markus Hitter: > Similar to NtReadFile and NtWriteFile, NtCreateFile should trace > return values on request. Providing the file handle allows to map > read/write requests to the corresponding file name. Seems very sensible. Your implementation looks OK, except for me not being sure whether status codes are usually printed decimal or hexadecimal. Please check. > Additionally, wrap a overly long line. Please don't put a style change and a functionality change that are independent into the same patch. Usual convention is to leave style of code parts one does not touch alone. If you really feel like that line should be changed, submit it as seperate patch. Regards, Michael Karcher

2 1

Re: Better reflect the current state of DrawIconEx
by Michael Karcher Aug. 18, 2008

Aug. 18, 2008

You indirectly asked for review of your patches. The comments below might be reasons your patch did not get accepted. Best way to handle ignored patches is to look over the patch again, and if one does not see something that can be improved, just ask on wine-devel or (even better) the IRC channel whether anyone can see something wrong with that patch. Am Montag, den 21.07.2008, 21:17 +0200 schrieb Markus Hitter: > diff --git a/dlls/user32/cursoricon.c b/dlls/user32/cursoricon.c > index a1ba570..eca9bfb 100644 > --- a/dlls/user32/cursoricon.c > +++ b/dlls/user32/cursoricon.c > @@ -1979,7 +1979,9 @@ BOOL WINAPI DrawIconEx( HDC hdc, INT x0, INT > y0, HICON hIcon, > if (istep) > FIXME_(icon)("Ignoring istep=%d\n", istep); > if (flags & DI_COMPAT) > - FIXME_(icon)("Ignoring flag DI_COMPAT\n"); > + FIXME_(icon)("DI_COMPAT flag unimplemented (before NT4.0)\n"); As this flag is ignored on newer windows versions, you might think about removing that check altogether instead of just changing the message. > + if (flags & DI_NOMIRROR) > + FIXME_(icon)("DI_NOMIRROR flag unimplemented (XP or later)\n"); As a wine user, I don't care which windows versions implement this flag. What I do care about is whether the program requested some functionality from wine it can't handle. So I would just show a message that says "DI_NOMIRROR flag unimplemented", and skip the version information. Regards, Michael Karcher

1 0