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

re: Hans Leidekker : msi: Support rollback of failed uninstalls.

by Hans Leidekker

On Wed, 2011-05-25 at 18:49 +0000, Dan Kegel wrote: > wow, msi has really come a long way! Thanks. In hindsight we should have implemented rollbacks early on, when installers still failed regularly ;-)

May 25, 2011

extern char **environ and msvc

by Stefan Dösinger

Hi, This is one of the last remaining hacks needed to compile the d3d parts of Wine with Visual Studio. Our definition of **environ is conflicting with one already in the Windows SDK headers: 1>..\..\wine\libs\wine\loader.c(54): warning C4273: '_environ': Inkonsistente DLL-Bindung. 1> C:\Program Files\Microsoft Visual Studio 10.0\VC\include\stdlib.h(299): Siehe vorherige Definition von '_environ' I'm sorry for the German, I don't know how to change this. Google finds only other clueless people. Either way it says something like "Inconsistent DLL binding" The line in stdlib.h declares _environ as _CRTIMP extern char ** _environ; /* pointer to environment table */ The compiler just writes a warning, but afterwards linking fails: 1>loader.obj : error LNK2001: Nicht aufgelöstes externes Symbol "__environ". The linker complains about an "unresolved external symbol __environ". Removing our declaration of environ fixes the warning, link error and libwine seems to work OK. I don't know what the proper fix is, please advise. Maybe this code shouldn't be compiled at all? I don't think I need the loader code on Windows. Stefan

May 25, 2011

Re: USB Device Support

by Scott

Hi, I've been working through my Dell Inspiron 1545 with Linux/Ubuntu 10.04 LTS experiences with ongoing reviews. I'd like to provide some help to others considering making the transition from Windows to Linux. I'm trying to get my Garmin GPS to talk to its Mapsource (running under WINE) application through a USB port. This allows tracks to be read from and written to the GPS itself. So far no luck. Can someone there please provide some advice? If this functionality isn't available yet then so be it. The Mapsource application runs fine it's just the USB communication that isn't working. Many thanks. Best - Scott

May 25, 2011

Re: [PATCH 1/2] ddraw: COM cleanup for IDirectDrawSurface3 iface

by Michael Stefaniuc

On 05/25/2011 08:37 PM, Stefan Dösinger wrote: > On Wednesday 25 May 2011 00:39:46 Ričardas Barkauskas wrote: >> My first try for COM cleanup. Please donėt be too harsh :) >> If this is too large for one patch, please suggest how to split it. >> >> This is not automatic clean up so I don't really expect it to be good on >> first try. > I can't find anything wrong with it on first sight, but I don't really know how > to manually review a patch that large with many repetitions. I guess I could > print them on a *lot* of paper and read them at the pool or something like > that. I normally read such a patch at least 3 times, more if I find some issues and have to manually fix them. Well "read"... after hundreds of such patches it isn't really a conscious read anymore. More like optical filtering looking for odd things; that's why I *hate* inconsistent coding style. > I don't see an easy way to split them up either :-/ . I guess we'll have to > trust Puk's script verification. The patch could be split up. What I do for such big and ugly patches is to first handle the unsafe_impl_from_IFace() part, then introduce impl_from_IFace() and only afterwards do the obj->lpVtl to obj->iface changes. Of course that really depends on the sizes of the resulting patches, but most of the times the unsafe_impl_from_IFace() part is a different patch as that isn't standard search and replace cookie cutter COM cleanup. > Just one thing: in unsafe_impl_from_IDirectDrawSurface*: > >> + if (iface->lpVtbl !=&ddraw_surface7_vtbl) return NULL; > I don't think apps can pass in their own ddraw object implementations. I've > never seen an app that does that or any document that suggests this is > possible and achives something. So I'd recommend to add a FIXME() or ERR() to > that case. An ERR because the most likely cause for such a behavior is that > our own ddrawex.dll screws up. This is the only other place where we have a > IDirectDrawSurface* implementation. Well in that case an assert() is the better option. It's done that way in other unsafe_impl_from_IFace like functions. And the program is likely to crash anyway later on when doing magic with the wrong pointer. > Or we could just remove the check and see if we ever crash because trying to > read surface implementation fields returns odd values. bye michael

May 25, 2011

re: Hans Leidekker : msi: Support rollback of failed uninstalls.

by Dan Kegel

wow, msi has really come a long way!

May 25, 2011

Re: [PATCH 1/2] ddraw: COM cleanup for IDirectDrawSurface3 iface

by Stefan Dösinger

On Wednesday 25 May 2011 00:39:46 Ričardas Barkauskas wrote: > My first try for COM cleanup. Please donėt be too harsh :) > If this is too large for one patch, please suggest how to split it. > > This is not automatic clean up so I don't really expect it to be good on > first try. I can't find anything wrong with it on first sight, but I don't really know how to manually review a patch that large with many repetitions. I guess I could print them on a *lot* of paper and read them at the pool or something like that. I don't see an easy way to split them up either :-/ . I guess we'll have to trust Puk's script verification. Just one thing: in unsafe_impl_from_IDirectDrawSurface*: > + if (iface->lpVtbl != &ddraw_surface7_vtbl) return NULL; I don't think apps can pass in their own ddraw object implementations. I've never seen an app that does that or any document that suggests this is possible and achives something. So I'd recommend to add a FIXME() or ERR() to that case. An ERR because the most likely cause for such a behavior is that our own ddrawex.dll screws up. This is the only other place where we have a IDirectDrawSurface* implementation. Or we could just remove the check and see if we ever crash because trying to read surface implementation fields returns odd values.

May 25, 2011

Re: [2/5] d3dx9: Generate effect instances from materials for mesh loading.

by Dan Kegel

On Wed, May 25, 2011 at 6:23 PM, Dylan Smith <dylan.ah.smith(a)gmail.com> wrote: > The layout doesn't group all values, param names, or defaults, so it > actually seems to be best described using loops. Does the the > following comment help summarize the layout? Yes. Patch looks more readable now, thanks. Maybe an assert at the bottom as a sanity check that the final output pointer is exactly at the end of the buffer would be good. - Dan

May 25, 2011

Re: [2/5] d3dx9: Generate effect instances from materials for mesh loading.

by Dylan Smith

Hi Dan, On Wed, May 25, 2011 at 10:31 AM, Dan Kegel <dank(a)kegel.com> wrote: > A comment or two about the layout of the output buffer might > be nice. > The layout doesn't group all values, param names, or defaults, so it actually seems to be best described using loops. Does the the following comment help summarize the layout? /* effects buffer layout: * * D3DXEFFECTINSTANCE effects[num_materials]; * for (effect in effects) * { * D3DXEFFECTDEFAULT defaults[effect.NumDefaults]; * for (default in defaults) * { * *default.pParamName; * *default.pValue; * } * } */ > Can materials and material_ptr be const pointers? ID3DXBuffer_GetBufferPointer doesn't take a const, so no for materials, but material_ptr can be const. The rest of the advice I took (see the attached patch). Thanks for the patch review.

May 25, 2011

Re: ole32/ole2: Don't call IDropTarget::QueryInterface() in RegisterDragDrop(). [whitespace fixed]

by Adam Martinson

On 05/25/2011 12:36 PM, Adam Martinson wrote: > On 05/24/2011 03:37 AM, Huw Davies wrote: >>> +static IDropTarget* WrapDropTarget(IDropTarget* inner) >>> +{ >>> + DropTargetWrapper* This = HeapAlloc(GetProcessHeap(), 0, >>> sizeof(*This)); >>> + >>> + if (This) >>> + { >>> + This->iface = (IDropTarget){&DropTargetWrapper_VTbl }; >> This->iface.lpVtbl =&DropTargetWrapper_VTbl; >> >>> - hr = CoMarshalInterface(stream,&IID_IDropTarget, unk, >>> MSHCTX_LOCAL, NULL, MSHLFLAGS_TABLESTRONG); >>> - IUnknown_Release(unk); >>> + hr = CoMarshalInterface(stream,&IID_IDropTarget, >>> (IUnknown*)wrapper, MSHCTX_LOCAL, NULL, MSHLFLAGS_TABLESTRONG); >>> >>> if(SUCCEEDED(hr)) >>> { >>> hr = create_map_from_stream(stream,&map); >>> if(SUCCEEDED(hr)) >>> { >>> - IDropTarget_AddRef(pDropTarget); >>> SetPropW(hwnd, prop_oledroptarget, pDropTarget); >> You probably want to set the window prop to the wrapper here. Either >> way you still need the AddRef (on the wrapper if you set that), it >> gets released in RevokeDragDrop. >> >> Huw. > > The AddRef is done in WrapDropTarget(), seems like the appropriate > place for it. Quite right on the rest though, thanks. Oops no, the window prop should be pDropTarget, not the wrapper. The wrapper is only used for the marshalling.

May 25, 2011

Re: ole32/ole2: Don't call IDropTarget::QueryInterface() in RegisterDragDrop(). [whitespace fixed]

by Adam Martinson

On 05/24/2011 03:37 AM, Huw Davies wrote: >> +static IDropTarget* WrapDropTarget(IDropTarget* inner) >> +{ >> + DropTargetWrapper* This = HeapAlloc(GetProcessHeap(), 0, sizeof(*This)); >> + >> + if (This) >> + { >> + This->iface = (IDropTarget){&DropTargetWrapper_VTbl }; > This->iface.lpVtbl =&DropTargetWrapper_VTbl; > >> - hr = CoMarshalInterface(stream,&IID_IDropTarget, unk, MSHCTX_LOCAL, NULL, MSHLFLAGS_TABLESTRONG); >> - IUnknown_Release(unk); >> + hr = CoMarshalInterface(stream,&IID_IDropTarget, (IUnknown*)wrapper, MSHCTX_LOCAL, NULL, MSHLFLAGS_TABLESTRONG); >> >> if(SUCCEEDED(hr)) >> { >> hr = create_map_from_stream(stream,&map); >> if(SUCCEEDED(hr)) >> { >> - IDropTarget_AddRef(pDropTarget); >> SetPropW(hwnd, prop_oledroptarget, pDropTarget); > You probably want to set the window prop to the wrapper here. Either > way you still need the AddRef (on the wrapper if you set that), it > gets released in RevokeDragDrop. > > Huw. The AddRef is done in WrapDropTarget(), seems like the appropriate place for it. Quite right on the rest though, thanks.

May 25, 2011