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

Re: [PATCH] wininet: Conformance test for InternetCrackUrlA (try 2)

by Detlef Riekenberg

On Mi, 2007-10-17 at 13:17 -0700, Nigel Liang wrote: > + urlCompa.dwStructSize = sizeof(urlCompa); > + bRet = InternetCrackUrlA(sUrl, 11, 0, &urlCompa); > + ZeroMemory(&urlCompa, sizeof(urlCompa)); > + ZeroMemory(&szBuf, sizeof(char)*1024); > + urlCompa.dwUserNameLength = 1024; It looks much better, when you avoid constant numbers. I suggest to use sizeof() instead. (but never use "sizeof()" in "ok()" or in "trace()".) Thanks for helping wine. -- By by ... Detlef

Oct. 18, 2007

Re: widl [3/4]: Remove printf format strings that aren't really format strings.

by Michael Stefaniuc

Dan Hipschman wrote: > On Thu, Oct 18, 2007 at 11:12:23AM +0200, Michael Stefaniuc wrote: >> Dan Hipschman wrote: >>> This is just a minor cleanup patch. Even if we're definitely sure a >>> string doesn't contain any % signs, it's still better not to use it as a >>> printf format string. >> fputs() would be a faster alternative to that. Though i doubt that >> matters in this case. > > You're right, I just use fputs so rarely that I sometimes forget it > exists. Feel free to change them, but like you said, it probably > doesn't matter much here. No, i don't like fputs(). It has the arguments the wrong way around which is more confusing then using fprintf() (of course if it's not in the hot path and performance critical). bye michael -- Michael Stefaniuc Tel.: +49-711-96437-199 Sr. Network Engineer Fax.: +49-711-96437-111 -------------------------------------------------------------------- Reg. Adresse: Red Hat GmbH, Hauptstätter Strasse 58, 70178 Stuttgart Handelsregister: Amtsgericht Stuttgart HRB 153243 Geschäftsführer: Brendan Lane, Charlie Peters, Michael Cunningham, Werner Knoblich

Oct. 18, 2007

Re: widl [4/4]: Detect conformant arrays of user types correctly.

by Dan Hipschman

On Thu, Oct 18, 2007 at 04:02:40PM +0100, Robert Shearman wrote: > Dan Hipschman wrote: > >@@ -1859,6 +1859,7 @@ static int get_struct_type(var_list_t *fields) > > case RPC_FC_OP: > > case RPC_FC_CARRAY: > > case RPC_FC_CVARRAY: > >+ case RPC_FC_BOGUS_ARRAY: > > has_pointer = 1; > > break; > > > >@@ -1897,15 +1898,9 @@ static int get_struct_type(var_list_t *fields) > > /* fallthru - treat it as complex */ > > > > /* as soon as we see one of these these members, it's bogus... */ > >- case RPC_FC_IP: > > case RPC_FC_ENCAPSULATED_UNION: > > case RPC_FC_NON_ENCAPSULATED_UNION: > >- case RPC_FC_TRANSMIT_AS: > >- case RPC_FC_REPRESENT_AS: > >- case RPC_FC_PAD: > >- case RPC_FC_EMBEDDED_COMPLEX: > > case RPC_FC_BOGUS_STRUCT: > >- case RPC_FC_BOGUS_ARRAY: > > return RPC_FC_BOGUS_STRUCT; > > } > > } > > > > Hi Dan, > > What's the reasoning behind this part of the patch? It seems to me like > most of the cases that you removed from becoming a complex structure are > actually valid. The hunk further up also confuses me - the presence of a > complex array shouldn't just make the resulting structure into a pointer > structure, it should make it into a complex structure, since it can't be > marshalled solely by using memcpy. Hi. For the first hunk: Fields that are actually declared as arrays (with [] notation) are handled above, so if we get to that part of the code and find a bogus array, it must be a pointer with a size_is attribute. For conformant arrays (bogus included) that are declared as pointers, MIDL treats them like pointers to arrays, instead of just embedded arrays. Since the field is listed as a pointer, all we need is PSTRUCT (or variant thereof). If I understand the RPC engine correctly, a PSTRUCT should get memcpy'd initially, but then for each pointer it goes through and marshals whatever it points to separately and then updates the value. For the second hunk: I moved the bogus array type for the reasons mentioned above. The other things there aren't real types. Pointers to FC_IP are handled above, and it should be impossible to have an interface itself as a field in a structure. FC_PAD and FC_EMBEDDED_COMPLEX aren't types, they're just format codes, and padding is handled above anyway. The transmit_as and represent_as types haven't been implemented yet, and when they are I'm guessing they will be done similarly to the way user_types are done. Thanks.

Oct. 18, 2007

Re: widl [3/4]: Remove printf format strings that aren't really format strings.

by Dan Hipschman

On Thu, Oct 18, 2007 at 11:12:23AM +0200, Michael Stefaniuc wrote: > Dan Hipschman wrote: > > This is just a minor cleanup patch. Even if we're definitely sure a > > string doesn't contain any % signs, it's still better not to use it as a > > printf format string. > fputs() would be a faster alternative to that. Though i doubt that > matters in this case. You're right, I just use fputs so rarely that I sometimes forget it exists. Feel free to change them, but like you said, it probably doesn't matter much here. Thanks.

Oct. 18, 2007

Re: Priority/severity of test suite hangs

by Maarten Lankhorst

Dan Kegel schreef: > I'm going to start filing bugs for anything that makes > the test suite hang, and I'm going to lash out and > give them unreasonably high priority and severity. > There's just no excuse for a test suite that can't > run to completion. > You might want to run the tests with WINEDEBUG=warn+heap, this will do 2 things: 1. Allocated memory will be filled with 0x55's. 2. Freed memory will be filled with 0xaa's. If something dereferences a structure it will use those magic values and then crash with values close to 0x55555555 or 0xaaaaaaaa which makes debugging easier. I just found a crash in crypt32 cert test that way, which would be a good bug report as well. Those issues are normally very hard to track down otherwise. Cheers, Maarten.

Oct. 18, 2007

Re: [2/3] hlink: Undefine the GetObject macro when compiling the Wine source to avoid errors.

by Jacek Caban

Hi Francois, Francois Gouget wrote: > > +/* GetObject is defined in wingdi.h as WINELIB_NAME_AW(GetObject), > + * which resolves to a compilation failure if WINE_NO_UNICODE_MACROS is > + * defined, but GetObject is used as a valid method name below, so we have > + * to undefine it in that case */ > +cpp_quote("#ifdef WINE_NO_UNICODE_MACROS") > +cpp_quote("#undef GetObject") > +cpp_quote("#endif") > + > /***************************************************************************** > * IHlinkBrowseContext interface > */ > I've sent a patch for it yesterday and it's applied now: http://source.winehq.org/git/wine.git/?a=commitdiff;h=a41cab9b52893f472de6c… I've also sent a patch for mshtml.idl patch (but my version didn't include import shtypes.idl): http://source.winehq.org/git/wine.git/?a=commitdiff;h=62894b488d0b03a922b80… Thanks, Jacek

Oct. 18, 2007

Re: widl [4/4]: Detect conformant arrays of user types correctly.

by Robert Shearman

Dan Hipschman wrote: > @@ -1859,6 +1859,7 @@ static int get_struct_type(var_list_t *fields) > case RPC_FC_OP: > case RPC_FC_CARRAY: > case RPC_FC_CVARRAY: > + case RPC_FC_BOGUS_ARRAY: > has_pointer = 1; > break; > > @@ -1897,15 +1898,9 @@ static int get_struct_type(var_list_t *fields) > /* fallthru - treat it as complex */ > > /* as soon as we see one of these these members, it's bogus... */ > - case RPC_FC_IP: > case RPC_FC_ENCAPSULATED_UNION: > case RPC_FC_NON_ENCAPSULATED_UNION: > - case RPC_FC_TRANSMIT_AS: > - case RPC_FC_REPRESENT_AS: > - case RPC_FC_PAD: > - case RPC_FC_EMBEDDED_COMPLEX: > case RPC_FC_BOGUS_STRUCT: > - case RPC_FC_BOGUS_ARRAY: > return RPC_FC_BOGUS_STRUCT; > } > } > Hi Dan, What's the reasoning behind this part of the patch? It seems to me like most of the cases that you removed from becoming a complex structure are actually valid. The hunk further up also confuses me - the presence of a complex array shouldn't just make the resulting structure into a pointer structure, it should make it into a complex structure, since it can't be marshalled solely by using memcpy. -- Rob Shearman

Oct. 18, 2007

Re: inkobj: Implement IClassFactory

by Alexandre Julliard

"John Klehm" <xixsimplicityxix(a)gmail.com> writes: > This patch is largely based on the code found in mshtml. What I did > was separate it into its own file and restyle it to my liking. Restyling it wasn't such a good idea I'm afraid, the original code is better without these ugly casts, and the Release function was correct, unlike yours. Also please don't name your files inkobj_something, they are in the inkobj directory already, no need to repeat that in the file name. -- Alexandre Julliard julliard(a)winehq.org

Oct. 18, 2007

Re: [PATCH] wininet: Substitute strchrW with memchrW in InternetCrackUrlW (try 2)

by Alexandre Julliard

Nigel Liang <ncliang(a)gmail.com> writes: > Previous patch had incorrect bounds for memchrW. There are other places in that function that assume that the string is null-terminated, please fix those too. -- Alexandre Julliard julliard(a)winehq.org

Oct. 18, 2007

Re: Priority/severity of test suite hangs

by Jakob Eriksson

Dan Kegel wrote: > I'm going to start filing bugs for anything that makes > the test suite hang, and I'm going to lash out and > give them unreasonably high priority and severity. > There's just no excuse for a test suite that can't > run to completion. > > +1

Oct. 18, 2007