December 2008 - Wine-devel - list.winehq.org

Re: [PATCH 14/14] msi: Add tests for MsiGetProductProperty.
by Paul Vriens 10 Dec '08

10 Dec '08

James Hawkins wrote: > --- > dlls/msi/tests/package.c | 232 ++++++++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 232 insertions(+), 0 deletions(-) > > > ------------------------------------------------------------------------ > > Hi James, These fail on Win9x as we are relying on some registry keys that are NT+ specific: 9574 lstrcpyA(keypath, "Software\\Microsoft\\Windows\\CurrentVersion\\"); 9575 lstrcatA(keypath, "Installer\\UserData\\S-1-5-18\\Products\\"); 9576 lstrcatA(keypath, prod_squashed); Can we skip these tests, like we do with some others: scm = OpenSCManager(NULL, NULL, GENERIC_ALL); if (!scm && (GetLastError() == ERROR_CALL_NOT_IMPLEMENTED)) { win_skip("Different registry keys on Win9x and WinMe\n"); return; } -- Cheers, Paul.

2 1

Re: [5/8] jscript: Implement the String.anchor() method.
by Jacek Caban 10 Dec '08

10 Dec '08

Hi Andrew, Andrew Nguyen wrote: > --- > dlls/jscript/string.c | 58 ++++++++++++++++++++++++++++++++++++++++++ > dlls/jscript/tests/api.js | 17 +++++++++++++ > 2 files changed, 73 insertions(+), 2 deletions(-) > +static HRESULT do_attribute_tag_format(DispatchEx *dispex, LCID lcid, WORD flags, DISPPARAMS *dp, + VARIANT *retv, jsexcept_t *ei, IServiceProvider *sp, const WCHAR *tagname, const WCHAR *attrname) +{ + static const WCHAR attrtagfmtW[] = {'<','%','s',' ','%','s','=','"','%','s','"','>','%','s','<','/','%','s','>',0}; + static const WCHAR undefinedW[] = {'u','n','d','e','f','i','n','e','d',0}; + StringInstance *string; + BSTR ret; + BSTR attrval = NULL; + DWORD len; + HRESULT hres = S_OK; + + TRACE("\n"); IMO it would be better to change FIXME to TRACE in String_anchor instead of adding TRACE here. You use this function in a few places, and this TRACE doesn't tell us, which function is called. + + if(!is_class(dispex, JSCLASS_STRING)) { + WARN("this is not a string object\n"); + hres = E_NOTIMPL; + goto cleanup; You may just return E_NOTIMPL here. + } + + string = (StringInstance*)dispex; + + if(retv) { + if(arg_cnt(dp)) { + hres = to_string(dispex->ctx, get_arg(dp, 0), ei, &attrval); You should call to_string even if retv is NULL. Although it looks like a nice optimization, we can't do this. Eg, if to_string throws exception, we have to throw it here. + if (FAILED(hres)) + goto cleanup; You may just return hres here. + } + + len = string->length + 2*strlenW(tagname) + strlenW(attrname) + 9; + + if (attrval) + len += SysStringLen(attrval); + else + len += sizeof(undefinedW)/sizeof(WCHAR) - 1; + + ret = SysAllocStringLen(NULL, len); + + if(!ret) + { + hres = E_OUTOFMEMORY; + goto cleanup; Perhaps it's just me, but in cases that don't make the code cleaner by using gotos, I'd avoid them. How about: if(ret) { sprintfW(ret, attrtagfmtW, tagname, attrname, (attrval ? attrval : undefinedW), string->str, tagname); V_VT(retv) = VT_BSTR; V_BSTR(retv) = ret; }else { hres = E_OUTOFMEMORY; } + } + + sprintfW(ret, attrtagfmtW, tagname, attrname, (attrval ? attrval : undefinedW), string->str, tagname); + + V_VT(retv) = VT_BSTR; + V_BSTR(retv) = ret; + } + cleanup: + SysFreeString(attrval); + return hres; +} Jacek

1 0

Re: [5/8] jscript: Implement the String.anchor() method.
by Jacek Caban 10 Dec '08

10 Dec '08

Hi Andrew, Andrew Nguyen wrote: > --- > dlls/jscript/string.c | 58 ++++++++++++++++++++++++++++++++++++++++++ > dlls/jscript/tests/api.js | 17 +++++++++++++ > 2 files changed, 73 insertions(+), 2 deletions(-) > +static HRESULT do_attribute_tag_format(DispatchEx *dispex, LCID lcid, WORD flags, DISPPARAMS *dp, + VARIANT *retv, jsexcept_t *ei, IServiceProvider *sp, const WCHAR *tagname, const WCHAR *attrname) +{ + static const WCHAR attrtagfmtW[] = {'<','%','s',' ','%','s','=','"','%','s','"','>','%','s','<','/','%','s','>',0}; + static const WCHAR undefinedW[] = {'u','n','d','e','f','i','n','e','d',0}; + StringInstance *string; + BSTR ret; + BSTR attrval = NULL; + DWORD len; + HRESULT hres = S_OK; + + TRACE("\n"); IMO it would be better to change FIXME to TRACE in String_anchor instead of adding TRACE here. You use this function in a few places, and this TRACE doesn't tell us, which function is called. + + if(!is_class(dispex, JSCLASS_STRING)) { + WARN("this is not a string object\n"); + hres = E_NOTIMPL; + goto cleanup; You may just return E_NOTIMPL here. + } + + string = (StringInstance*)dispex; + + if(retv) { + if(arg_cnt(dp)) { + hres = to_string(dispex->ctx, get_arg(dp, 0), ei, &attrval); You should call to_string even if retv is NULL. Although it looks like a nice optimization, we can't do this. Eg, if to_string throws exception, we have to throw it here. + if (FAILED(hres)) + goto cleanup; You may just return hres here. + } + + len = string->length + 2*strlenW(tagname) + strlenW(attrname) + 9; + + if (attrval) + len += SysStringLen(attrval); + else + len += sizeof(undefinedW)/sizeof(WCHAR) - 1; + + ret = SysAllocStringLen(NULL, len); + + if(!ret) + { + hres = E_OUTOFMEMORY; + goto cleanup; Perhaps it's just me, but in cases that don't make the code cleaner by using gotos, I'd avoid them. How about: if(ret) { sprintfW(ret, attrtagfmtW, tagname, attrname, (attrval ? attrval : undefinedW), string->str, tagname); V_VT(retv) = VT_BSTR; V_BSTR(retv) = ret; }else { hres = E_OUTOFMEMORY; } + } + + sprintfW(ret, attrtagfmtW, tagname, attrname, (attrval ? attrval : undefinedW), string->str, tagname); + + V_VT(retv) = VT_BSTR; + V_BSTR(retv) = ret; + } + cleanup: + SysFreeString(attrval); + return hres; +}

1 0

Re: oleaut32: added PICTYPE_NONE and PICTYPE_UNINITIALIZED to IPicture::Render with test (try3)
by Paul Vriens 10 Dec '08

10 Dec '08

Nikolay Sivov wrote: > Changelog(try3): > - fixed identation in testlist > Changelog(try2): > - added PICTYPE_NONE and PICTYPE_UNINITIALIZED to IPicture::Render with test > - added test for zero dimensions > > P.S. Render with this types spotted in traces of bugs 6799 and 10050 > >>From 3386e8674380390c3b60c6247443c605ffe6ff6a Mon Sep 17 00:00:00 2001 > From: Nikolay Sivov <bunglehead(a)gmail.com> > Date: Wed, 10 Dec 2008 13:41:09 +0300 > Subject: added PICTYPE_NONE and PICTYPE_UNINITIALIZED to IPicture::Render > > --- > dlls/oleaut32/olepicture.c | 8 +++++ > dlls/oleaut32/tests/olepicture.c | 64 ++++++++++++++++++++++++++++++++++++++ > 2 files changed, 72 insertions(+), 0 deletions(-) > > diff --git a/dlls/oleaut32/olepicture.c b/dlls/oleaut32/olepicture.c > index 498ee53..bdce729 100644 > --- a/dlls/oleaut32/olepicture.c > +++ b/dlls/oleaut32/olepicture.c > @@ -637,6 +637,10 @@ static HRESULT WINAPI OLEPictureImpl_Render(IPicture *iface, HDC hdc, > TRACE("prcWBounds (%d,%d) - (%d,%d)\n", prcWBounds->left, prcWBounds->top, > prcWBounds->right, prcWBounds->bottom); > > + if(cx == 0 || cy == 0 || cxSrc == 0 || cySrc == 0){ > + return CTL_E_INVALIDPROPERTYVALUE; > + } > + > /* > * While the documentation suggests this to be here (or after rendering?) > * it does cause an endless recursion in my sample app. -MM 20010804 > @@ -644,6 +648,10 @@ static HRESULT WINAPI OLEPictureImpl_Render(IPicture *iface, HDC hdc, > */ > > switch(This->desc.picType) { > + case PICTYPE_UNINITIALIZED: > + case PICTYPE_NONE: > + /* nothing to do */ > + return S_OK; > case PICTYPE_BITMAP: > { > HBITMAP hbmpOld; > diff --git a/dlls/oleaut32/tests/olepicture.c b/dlls/oleaut32/tests/olepicture.c > index f81c419..9e33121 100644 > --- a/dlls/oleaut32/tests/olepicture.c > +++ b/dlls/oleaut32/tests/olepicture.c > @@ -25,6 +25,7 @@ > #include <float.h> > > #define COBJMACROS > +#define NONAMELESSUNION > > #include "wine/test.h" > #include <windef.h> > @@ -587,6 +588,68 @@ static void test_enhmetafile(void) > GlobalFree(hglob); > } > > +static void test_Render(void) > +{ > + IPicture *pic; > + HRESULT hres; > + short type; > + PICTDESC desc; > + HDC hdc = GetDC(0); > + > + /* test IPicture::Render return code on uninitialized picture */ > + OleCreatePictureIndirect(NULL, &IID_IPicture, TRUE, (VOID**)&pic); > + hres = IPicture_get_Type(pic, &type); > + ok(hres == S_OK, "IPicture_get_Type does not return S_OK, but 0x%08x\n", hres); > + ok(type == PICTYPE_UNINITIALIZED, "Expected type = PICTYPE_UNINITIALIZED, got = %d\n", type); > + /* zero dimensions */ > + hres = IPicture_Render(pic, hdc, 0, 0, 0, 0, 0, 0, 0, 0, NULL); > + ole_expect(hres, CTL_E_INVALIDPROPERTYVALUE); > + hres = IPicture_Render(pic, hdc, 0, 0, 10, 10, 0, 0, 10, 0, NULL); > + ole_expect(hres, CTL_E_INVALIDPROPERTYVALUE); > + hres = IPicture_Render(pic, hdc, 0, 0, 10, 10, 0, 0, 0, 10, NULL); > + ole_expect(hres, CTL_E_INVALIDPROPERTYVALUE); > + hres = IPicture_Render(pic, hdc, 0, 0, 10, 10, 0, 0, 0, 0, NULL); > + ole_expect(hres, CTL_E_INVALIDPROPERTYVALUE); > + hres = IPicture_Render(pic, hdc, 0, 0, 0, 10, 0, 0, 10, 10, NULL); > + ole_expect(hres, CTL_E_INVALIDPROPERTYVALUE); > + hres = IPicture_Render(pic, hdc, 0, 0, 10, 0, 0, 0, 10, 10, NULL); > + ole_expect(hres, CTL_E_INVALIDPROPERTYVALUE); > + hres = IPicture_Render(pic, hdc, 0, 0, 0, 0, 0, 0, 10, 10, NULL); > + ole_expect(hres, CTL_E_INVALIDPROPERTYVALUE); > + /* nonzero dimensions, PICTYPE_UNINITIALIZED */ > + hres = IPicture_Render(pic, hdc, 0, 0, 10, 10, 0, 0, 10, 10, NULL); > + ole_expect(hres, S_OK); > + IPicture_Release(pic); > + > + desc.cbSizeofstruct = sizeof(PICTDESC); > + desc.picType = PICTYPE_ICON; > + desc.u.icon.hicon = LoadIcon(NULL, IDI_APPLICATION); > + if(!desc.u.icon.hicon){ > + win_skip("LoadIcon failed. Skipping...\n"); Shouldn't you also use ReleaseDC() here for consistency sake? > + return; > + } > + > + OleCreatePictureIndirect(&desc, &IID_IPicture, TRUE, (VOID**)&pic); > + /* zero dimensions, PICTYPE_ICON */ > + hres = IPicture_Render(pic, hdc, 0, 0, 0, 0, 0, 0, 0, 0, NULL); > + ole_expect(hres, CTL_E_INVALIDPROPERTYVALUE); > + hres = IPicture_Render(pic, hdc, 0, 0, 10, 10, 0, 0, 10, 0, NULL); > + ole_expect(hres, CTL_E_INVALIDPROPERTYVALUE); > + hres = IPicture_Render(pic, hdc, 0, 0, 10, 10, 0, 0, 0, 10, NULL); > + ole_expect(hres, CTL_E_INVALIDPROPERTYVALUE); > + hres = IPicture_Render(pic, hdc, 0, 0, 10, 10, 0, 0, 0, 0, NULL); > + ole_expect(hres, CTL_E_INVALIDPROPERTYVALUE); > + hres = IPicture_Render(pic, hdc, 0, 0, 0, 10, 0, 0, 10, 10, NULL); > + ole_expect(hres, CTL_E_INVALIDPROPERTYVALUE); > + hres = IPicture_Render(pic, hdc, 0, 0, 10, 0, 0, 0, 10, 10, NULL); > + ole_expect(hres, CTL_E_INVALIDPROPERTYVALUE); > + hres = IPicture_Render(pic, hdc, 0, 0, 0, 0, 0, 0, 10, 10, NULL); > + ole_expect(hres, CTL_E_INVALIDPROPERTYVALUE); > + IPicture_Release(pic); > + > + ReleaseDC(NULL, hdc); -- Cheers, Paul.

2 1

Re: oleaut32: added PICTYPE_NONE and PICTYPE_UNINITIALIZED to IPicture::Render with test
by Alexandre Julliard 10 Dec '08

10 Dec '08

Nikolay Sivov <bunglehead(a)gmail.com> writes: > Changelog: > - added PICTYPE_NONE and PICTYPE_UNINITIALIZED to IPicture::Render with test > - added test for zero dimensions > - remove some tabs from testfile Please don't change formatting, especially with an incorrect tab size. -- Alexandre Julliard julliard(a)winehq.org

3 3

Re: [5/5] winex11.drv: Fix color conversion for 16 bpp cursors.
by Alexandre Julliard 10 Dec '08

10 Dec '08

Henri Verbeet <hverbeet(a)codeweavers.com> writes: > case 16: > - /* BGR, 5 red, 6 green, 5 blue */ > - *pixel_ptr = *xor_ptr * 0x1f; > - *pixel_ptr |= (*xor_ptr & 0xe0) << 3; > + /* [gggbbbbb][rrrrrggg] -> [xxxxxxxx][rrrrr000][gggggg00][bbbbb000] */ It seems to me that zero-padding isn't quite right, you should expand the values linearly. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: winetest: fix a compiler warning
by Vincent Povirk 10 Dec '08

10 Dec '08

These are addressing some very weird cases. They should be an improvement, but I wonder if we can't do better. > void xprintf (const char *fmt, ...) > { > va_list ap; >- size_t size; >+ size_t size = 0; > DWORD written; It would make more sense, IMO, to update the vstrfmtmake function to also set *lenp to 0, if lenp is set, when it runs out of memory. Winetest uses that function indirectly in other places, and one has a similar problem. (Of course, if we did run out of memory, it's a pretty dismal situation, and the returned string will be NULL, but at least xprintf will silently fail instead of crashing.) > extract_test (struct wine_test *test, const char *dir, LPTSTR res_name) > { > BYTE* code; >- DWORD size; >+ DWORD size = 0; For this to matter, the extract_rcdata call has to fail. This is, again, a pretty dismal situation. In this case, we'll later be writing data starting at NULL, and of an undefined, or with this change 0, length. I don't know if that actually works, and it may be better to give up early. Vincent Povirk

1 0

Patchwatcher status: offline
by Dan Kegel 09 Dec '08

09 Dec '08

My house is being rewired, so Patchwatcher is offline for a day.

1 0

re: App hang on creating modal dialog
by Dan Kegel 09 Dec '08

09 Dec '08

Jeff wrote: >I'm trying to solve my bug 16346 >http://bugs.winehq.org/show_bug.cgi?id=16346 >... >I'm trying to figure out where in the wine code I should look for a >system that might be eating all of the messages intended for my >application. Can anyone point me in the right direction? I dunno, but that test case is nice and small. Can you turn it into a wine conformance test? That will make it even more likely somebody helps find a solution... - Dan

2 2

Re: Одговор: Regression in 'Environment Bump Mapping' test (3DMark2001SE) with wine-1.1.10
by Henri Verbeet 09 Dec '08

09 Dec '08

2008/12/9 Milan Kostić <smoki00790(a)gmail.com>: > Henry, with this patch 'Environment Bump Mapping' test is looking good, but > 'fixme:d3d_shader:register_for_arg Unknown source argument 255' is > still there - continous in many cases, not just in 3DMark2001SE... Is > this fixme useless, when this works? > Stefan has a fix for that.

2 1