July 2011 - Wine-Devel - list.winehq.org

Re: d3dx9_36: Push down the scope of two variables. (RESEND)

by Tony Wasserka

Am 05.07.2011 20:33, schrieb Gerald Pfeifer: > On Tue, 5 Jul 2011, Alexandre Julliard wrote: >>> This does not fix a bug or add a feature, but it makes the coder easier >>> to follow. Narrower scope tends to be a good thing for variables, both >>> for human readers and compilers (though modern ones should not need the >>> hint). >> Not necessarily, that's a matter of taste (IOW, not something you should >> "fix" in other people's code). > The reason I came up with this patch is that trying to debug something > I got lost a bit, and reducing the scope of these variables really helped > make things easier to follow. > > > When you are referring to "other people's code", according to git log > Tony has been the primary contributor, so that would be his call then? > > Tony, this is > http://www.winehq.org/pipermail/wine-patches/2011-July/103866.html > -- what's your take? > > Gerald I doubt this change is critical enough to justify any need for the original author's opinion :P I personally prefer narrow scope as well usually... if you want to change it, sure, I'm unlikely to touch that code ever again anyway.. Best regards, Tony

July 5, 2011

Re: [PATCH 1/5] d3dx9_36: Implement D3DXHANDLE more robustly

by Henri Verbeet

On 5 July 2011 23:45, Travis Athougies <iammisc(a)gmail.com> wrote: > Fixes bug 26583. > > Allows deadspace 2 to run without native dlls. This contains no useful information about what issue this is supposed to fix, unless you expect people to actually debug it themselves first. And I even went through the trouble to lookup the bug. > +static WORD new_handle_with_desc_and_name(ID3DXConstantTableImpl* This, D3DXCONSTANT_DESC* desc, LPCSTR name, LPCSTR full_name) > +{ > + ++This->handle_count; > + if (This->handles) > + This->handles = HeapReAlloc(GetProcessHeap(), 0, This->handles, sizeof(ctab_handle) * This->handle_count); > + else > + This->handles = HeapAlloc(GetProcessHeap(), 0, sizeof(ctab_handle) * This->handle_count); > + > + This->handles[This->handle_count - 1].desc = *desc; > + This->handles[This->handle_count - 1].desc.Name = strdup(name); > + This->handles[This->handle_count - 1].full_name = strdup(full_name); > + > + return This->handle_count; > +} That's just terrible.

July 5, 2011

Re: d3dx9_36: Push down the scope of two variables. (RESEND)

by Henri Verbeet

I think the point is more that (in general) this kind of patch is unlikely to be accepted unless you're actively maintaining the code.

July 5, 2011

Re: [PATCH 1/2] d3dx9/line: Implement Begin and End methods. (try 2)

by John Klehm

On Tue, Jul 5, 2011 at 2:46 PM, Charles Welton <charleswfb(a)gmail.com> wrote: > Is there a reason why this patch and it's brother weren't merged in yet? > This has been sitting for a few days and no comments about it yet. > According to http://source.winehq.org/patches/ #75906 it has yet to be looked at by AJ. Just have to wait it out. --John

July 5, 2011

Re: [PATCH 1/2] d3dx9/line: Implement Begin and End methods. (try 2)

by Charles Welton

Is there a reason why this patch and it's brother weren't merged in yet? This has been sitting for a few days and no comments about it yet. On 06/29/2011 07:08 PM, Charles Welton wrote: > Since the last patch spent some time without comments and wasn't > commited, I'm resending. > > Try 2 because of TRACE calls and whitespace fixes. > > --- > dlls/d3dx9_36/line.c | 73 > +++++++++++++++++++++++++++++++++++++++++++++++--- > 1 files changed, 69 insertions(+), 4 deletions(-) > > -- Charles Welton

July 5, 2011

Re: d3dx9_36: Push down the scope of two variables. (RESEND)

by Gerald Pfeifer

On Tue, 5 Jul 2011, Alexandre Julliard wrote: >> This does not fix a bug or add a feature, but it makes the coder easier >> to follow. Narrower scope tends to be a good thing for variables, both >> for human readers and compilers (though modern ones should not need the >> hint). > Not necessarily, that's a matter of taste (IOW, not something you should > "fix" in other people's code). The reason I came up with this patch is that trying to debug something I got lost a bit, and reducing the scope of these variables really helped make things easier to follow. When you are referring to "other people's code", according to git log Tony has been the primary contributor, so that would be his call then? Tony, this is http://www.winehq.org/pipermail/wine-patches/2011-July/103866.html -- what's your take? Gerald

July 5, 2011

Re: [4/9] msxml3: Implement IXMLDOMNodeList::QueryInterface(IEnumVARIANT).

by Adam Martinson

On 07/05/2011 11:23 AM, Nikolay Sivov wrote: > On Tue, Jul 5, 2011 at 8:11 PM, Adam Martinson > <amartinson(a)codeweavers.com> wrote: >> --- >> dlls/msxml3/nodelist.c | 33 ++++++++++++++++++++++++++++++++- >> dlls/msxml3/selection.c | 34 ++++++++++++++++++++++++++++++++-- >> 2 files changed, 64 insertions(+), 3 deletions(-) >> >> > + else if ( IsEqualGUID( riid,&IID_IEnumVARIANT ) ) > + { > + if (!This->enumv&& > + (This->enumv = create_nodelist_enum(&This->IXMLDOMNodeList_iface))) > + IXMLDOMNodeList_Release(iface); /* > create_nodelist_enum() adds a ref, and we don't here. */ > + > + if (!This->enumv) > + { > + *ppvObject = NULL; > + return E_OUTOFMEMORY; > + } > + > + IEnumVARIANT_AddRef(This->enumv); > + *ppvObject = This->enumv; > + return S_OK; > + } > > Shouldn't this->enumv be protected somehow to work with multiple > threads? I know that so called FreeThreaded* thing is only reported to > be supported, but still. Maybe something like pointer exchange call > will work. I like that idea, thanks!

July 5, 2011

Re: [PATCH] wscript: implemented Host_get_BuildVersion

by Paul Vriens

On 07/04/2011 11:28 PM, Michał Ziętek wrote: > +ok(typeof(WScript.BuildVersion) === "number", "typeof(WScript.BuldVersion) = " + typeof(WScript.BuldVersion)); Patch is already committed but there is a typo here 'BuldVersion' instead of 'BuildVersion'. -- Cheers, Paul.

July 5, 2011

Re: [4/9] msxml3: Implement IXMLDOMNodeList::QueryInterface(IEnumVARIANT).

by Nikolay Sivov

On Tue, Jul 5, 2011 at 8:11 PM, Adam Martinson <amartinson(a)codeweavers.com> wrote: > --- > dlls/msxml3/nodelist.c | 33 ++++++++++++++++++++++++++++++++- > dlls/msxml3/selection.c | 34 ++++++++++++++++++++++++++++++++-- > 2 files changed, 64 insertions(+), 3 deletions(-) > > + else if ( IsEqualGUID( riid, &IID_IEnumVARIANT ) ) + { + if (!This->enumv && + (This->enumv = create_nodelist_enum(&This->IXMLDOMNodeList_iface))) + IXMLDOMNodeList_Release(iface); /* create_nodelist_enum() adds a ref, and we don't here. */ + + if (!This->enumv) + { + *ppvObject = NULL; + return E_OUTOFMEMORY; + } + + IEnumVARIANT_AddRef(This->enumv); + *ppvObject = This->enumv; + return S_OK; + } Shouldn't this->enumv be protected somehow to work with multiple threads? I know that so called FreeThreaded* thing is only reported to be supported, but still. Maybe something like pointer exchange call will work.

July 5, 2011

Re: winedevice: eliminate the driver_name global variable

by Damjan Jovanovic

On Tue, Jul 5, 2011 at 4:23 PM, Alexandre Julliard <julliard(a)winehq.org> wrote: > Damjan Jovanovic <damjan.jov(a)gmail.com> writes: > >> Changelog: >> * winedevice: eliminate the driver_name global variable >> >> This is part of general refactoring to make the driver loading code reusable. > > Have you fixed it since last time or is it the same patch? Either way > it's still crashing. > > -- > Alexandre Julliard > julliard(a)winehq.org > It's a subset of the previous patch. I've finally found the problem, it only shows up with a clean wineprefix. Damjan

July 5, 2011