March 2010 - Wine-devel - list.winehq.org

Verbose explanation about the cygwin issue
by GOUJON Alexandre 26 Mar '10

26 Mar '10

Alexandre, (I hope I'm not being direspectful) your patch (561a3e643ea8b35adfc032d3afa979c622fcec23) did not work. I know that there is no Cygwin VM in WineTestBot yet to test such a patch. I think I was not very clear about the issue on cygwin so this time I'll be more .. verbose. Here is the relevant call stack of winegcc on cygwin : 1093 _int main(int argc, char **argv) 1373 _ else if (linking) build(&opts); 559 _static void build(struct options* opts) 722 _ strarray_add(link_args, mingw_unicode_hack(opts)); 547 _static const char *mingw_unicode_hack( struct options *opts ) 556 _ return compile_to_object( opts, main_stub, NULL ); 457 _static const char* compile_to_object(struct options* opts, const char* file, const char* lang) 470 _ compile(&copts, lang); 309 _static void compile(struct options* opts, const char* lang) ...here your patch didn't change anything because it hasn't been read 453 _ spawn(opts->prefix, comp_args, 0); so winegcc [1] calls ccache gcc-4 -D__WINE__ -c -o cmd.exe-2VhiZq-6BngcL.o cmd.exe-2VhiZq.c -I../../include that leads to cmd.exe-2VhiZq.c: In function ‘main’: cmd.exe-2VhiZq.c:5: error: ‘__wargv’ undeclared (first use in this function) cmd.exe-2VhiZq.c:5: error: (Each undeclared identifier is reported only once cmd.exe-2VhiZq.c:5: error: for each function it appears in.) winegcc: ccache failed make: *** [cmd.exe] Error 2 To fix it, we need to add msvcrt include (not to link with msvcrt : it's a .o) ccache gcc-4 -D__WINE__ -c -o cmd.exe-iqnSoR-q4unrB.o cmd.exe-iqnSoR.c -I../../include -I../../include/msvcrt/ succeeds. At this point, we return from mingw_unicode_hack to line 722. And winegcc goes on: 779 _ if (!opts->shared && (opts->use_msvcrt || opts->unicode_app)) strarray_add(link_args, "-lmsvcrt"); ...your patch 781 _if (res_o_name) compile_resources_to_object( opts, resources, res_o_name ); ...the previous line works, no errors but 783 _ spawn(opts->prefix, link_args, 0); ..fails Indeed, the linker [2] wants to link with msvcrt but don't know where the library is : /usr/lib/gcc/i686-pc-cygwin/4.3.4/../../../../i686-pc-cygwin/bin/ld: cannot find -lmsvcrt collect2: ld returned 1 exit status winegcc: ccache failed make: *** [cmd.exe] Error 2 Finally, adding -L option [3], all's right. Sorry for this looong mail. I think I won't send a patch but if you ask me. So thanks again and have a nice week end. --- [1] ../../tools/winegcc/winegcc -B../../tools/winebuild --sysroot=../.. -mconsole -municode batch.o builtins.o directory.o wcmdmain.o Cs.res Da.res De.res En.res Es.res Fr.res It.res Ja.res Ko.res Lt.res Nl.res No.res Pl.res Pt.res Ru.res Si.res Tr.res wcmdrc.res -o cmd.exe -lshell32 -luser32 -ladvapi32 -lkernel32 -lwine ../../libs/port/libwine_port.a [2] ccache gcc-4 -mconsole -o cmd.exe cmd.exe-iqnSoR-q4unrB.o -L../../dlls -L../../libs/wine batch.o builtins.o directory.o wcmdmain.o cmd.exe-XiDu0i.res.o -L../../dlls/shell32 -lshell32 -L../../dlls/user32 -luser32 -L../../dlls/advapi32 -ladvapi32 -L../../dlls/kernel32 -lkernel32 -L../../libs/wine -lwine -L../../libs/port -lwine_port -L../../dlls/winecrt0 -lwinecrt0 -lmsvcrt [3] ccache gcc-4 -mconsole -o cmd.exe cmd.exe-mfRWfQ-uwdRsu.o -L../../dlls -L../../libs/wine batch.o builtins.o directory.o wcmdmain.o cmd.exe-1DumDa.res.o -L../../dlls/shell32 -lshell32 -L../../dlls/user32 -luser32 -L../../dlls/advapi32 -ladvapi32 -L../../dlls/kernel32 -lkernel32 -L../../libs/wine -lwine -L../../libs/port -lwine_port -L../../dlls/winecrt0 -lwinecrt0 -lmsvcrt -L../../dlls/msvcr

5 9

Latest Git Fails tools/install
by chris ahrendt 26 Mar '10

26 Mar '10

> Compiling Wine. Grab a lunch or two, rent a video, or whatever, > in the meantime... > > config.status: executing Makefile commands > cat: Make.tmp: No such file or directory > config.status: error: could not create Makefile > make: *** [Makefile] Error 1 > config.status: executing Makefile commands > cat: Make.tmp: No such file or directory > config.status: error: could not create Makefile > make: *** [Makefile] Error 1 > > Compilation failed, aborting install. Ideas? Chris

5 6

Re: [PATCH 4/4] mshtml: Added IHTMLElement:removeAttribute tests.
by Paul Vriens 26 Mar '10

26 Mar '10

Hi Jacek, On 03/24/2010 08:01 PM, Jacek Caban wrote: > +function test_removeAttribute(e) { > + ok(e.removeAttribute('nonexisting') === false, "removeAttribute('nonexisting') didn't return false"); > + > + e.title = "title"; > + ok(e.removeAttribute('title') === true, "removeAttribute('title') didn't return true"); > + ok(e.title === "", "e.title = " + e.title); > + ok(("title" in e) === true, "title is not in e"); (commenting that one) > + > + e["myattr"] = "test"; > + ok(e.removeAttribute('myattr') === true, "removeAttribute('myattr') didn't return true"); > + ok(e["myattr"] === undefined, "e['myattr'] = " + e['myattr']); (and that one) > + ok(("myattr" in e) === false, "myattr is in e"); (and that one makes the tests run again). > + > +} These new tests introduce timeouts on IE5 (and lower?). If I just open the jstest.html in a IE5 browser I get a warning for some of the lines: > + ok(("title" in e) === true, "title is not in e"); > + ok(("myattr" in e) === false, "myattr is in e"); shows: Line: 14 Char: 17 (18 for that second one of course) Error: Expected ')' -- Cheers, Paul.

2 1

Re: 3/4 wined3d: add color_fill to blit_shader [try 2]
by Henri Verbeet 26 Mar '10

26 Mar '10

That's more reasonable, but you still have a NULL function pointer in arbfp_blit for color_fill(), and blit_operation is unused.

1 0

Re: 3/4 wined3d: add color_fill to blit_shader
by Henri Verbeet 25 Mar '10

25 Mar '10

On 25 March 2010 23:04, Roderick Colenbrander <thunderbird2k(a)gmail.com> wrote: > This patch adds a first part of the new blit_shader infrastructure. In > a few days I plan to submit more parts of it which will extend the > blit_shader and rewrite parts in the end. > This is patch is pretty hacky. > +static const struct blit_shader *blitters[] = > +{ > + &ffp_blit, > + &cpu_blit > +}; This doesn't make much sense here. You'll probably want to make it a variable local to surface_select_blitter() as a start, but eventually it should be stored in the device / adapter. > +const struct blit_shader *surface_select_blitter(const struct wined3d_gl_info *gl_info, IWineD3DSurfaceImpl *src, IWineD3DSurfaceImpl *dst, enum blit_operation op) Why do we have both this and select_blit_implementation() now? Also, static. > + switch(op) > + { > + case BLIT_OP_COLOR_FILL: > + return TRUE; > + default: > + return FALSE; > + } This is a silly way to write "return op == BLIT_OP_COLOR_FILL;". > +const struct blit_shader cpu_blit = { > + NULL, /* alloc_private */ > + NULL, /* free_private */ > + NULL, /* set_shader */ > + NULL, /* unset_shader */ > + NULL, /* color_fixup_supported */ > + cpu_blit_can_blit, > + cpu_blit_color_fill > }; You can't do that, callers don't check these for NULL. > +typedef enum blit_operation > +{ > + BLIT_OP_COLOR_FILL=1, > + BLIT_OP_BLIT_SYSMEM_TO_DRAWABLE=2, > + BLIT_OP_BLIT_DRAWABLE_TO_TEXTURE=3 > +} blit_operation; Besides being unused, the distinction between "SYSMEM_TO_DRAWABLE", "DRAWABLE_TO_TEXTURE", and other potential variants is an implementation detail callers shouldn't care about. Also, we don't care about the exact values of the enum elements, and the typedef only obscures the type of "blit_operation". > BOOL (*color_fixup_supported)(const struct wined3d_gl_info *gl_info, struct color_fixup_desc fixup); > + BOOL (*can_blit)(const struct wined3d_gl_info *gl_info, blit_operation op, IWineD3DSurfaceImpl *src_surface, IWineD3DSurfaceImpl *dst_surface); The functionality of "can_blit" is a superset of "color_fixup_supported". > + IWineD3DSurface_GetContainer((IWineD3DSurface*)dst_surface, &IID_IWineD3DSwapChain, (void **)&dst_swapchain); > + if(dst_swapchain) IWineD3DSwapChain_Release((IWineD3DSwapChain *) dst_swapchain); That's three different styles of doing a cast, in two lines of code. And code that's supposedly being rewritten at that. At this point I don't even care about the exact style you're using (although I do have a preference), but try to at least care about consistency. Same goes for these: > + if(dst_swapchain) IWineD3DSwapChain_Release((IWineD3DSwapChain *) dst_swapchain); > + switch(op) > + if ((IWineD3DSurface*)dst_surface != device->render_targets[0] && > +const struct blit_shader *surface_select_blitter(const struct wined3d_gl_info *gl_info, IWineD3DSurfaceImpl *src, IWineD3DSurfaceImpl *dst, enum blit_operation op) This line is a bit long for my taste. I think I have relatively large screens, but this takes up over half a screen. Personally I think 120 characters is a reasonable upper limit, but there are probably plenty of people that would be happier with 80. Essentially, I think you're trying to take too large steps. E.g. splitting of functionality into cpu_blit_color_fill(), ffp_blit_color_fill(), etc. is ok, hacking it into struct blit_shader isn't. Extending color_fixup_supported() into something like blit_supported() could also be a reasonable patch, on its own, but trying to do it all at the same time becomes hacky pretty quickly.

1 0

Re: 1/4 wined3d: Move argb_to_fmt to utils.c as a helper function to be used in more places.
by Stefan Dösinger 25 Mar '10

25 Mar '10

Am Donnerstag 25 März 2010 23:02:33 schrieb Roderick Colenbrander: > +DWORD color_convert_argb_to_fmt(DWORD color, WINED3DFORMAT destfmt) This only works with formats that are encoded in integers with 32 bit or less. Do we care about this limitation?

3 2

Re: [PATCH 6/8] msvcrt: Added _strtoi64 implementation (try4)
by Alexandre Julliard 25 Mar '10

25 Mar '10

Piotr Caban <piotr(a)codeweavers.com> writes: > --- > dlls/msvcr80/msvcr80.spec | 4 +- > dlls/msvcr90/msvcr90.spec | 4 +- > dlls/msvcrt/msvcrt.h | 44 ++++++++++++++++ > dlls/msvcrt/msvcrt.spec | 4 +- > dlls/msvcrt/string.c | 80 ++++++++++++++++++++++++++++ > dlls/msvcrt/tests/string.c | 123 > ++++++++++++++++++++++++++++++++++++++++++++ > include/msvcrt/crtdefs.h | 52 ++++++++++++++++++ > 7 files changed, 305 insertions(+), 6 deletions(-) This one doesn't build on Mingw either, because of crtdefs.h conflicts. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [2/2] dlls/crypt32: unit test PFXImportCertStore()
by Juan Lang 25 Mar '10

25 Mar '10

Hi Philippe, in addition to the question I had on patch 1/2, I'd like to know, what is the purpose of this test? +static void test_createCertificateStore(void) +{ + HCERTSTORE certStore = NULL; + + certStore = CertOpenStore(CERT_STORE_PROV_SYSTEM, 0, (HCRYPTPROV)NULL, + CERT_SYSTEM_STORE_CURRENT_USER, L"Test Store"); + ok(certStore != NULL, "could not open the system certificate store\n"); + if (certStore) + { + CertCloseStore(certStore, 0); + } This is already covered by tests in store.c, and you're not using "Test Store" elsewhere in the tests, so I don't see why this is necessary. For future reference, L"" string constants aren't portable, don't use them in the tests. And, please use CertOpenStoreW if you're going to use a wide-string parameter to it, or CertOpenStoreA if you're going to use an ASCII string. Thanks, --Juan

2 1

Re: Patch: Bug 19668 cmd /c doesn't wait before exiting
by André Hentschel 25 Mar '10

25 Mar '10

Hi Anh and Welcome to Wine! It seems your patch got mangled in transit, e.g. git apply says: fatal: corrupt patch at line 33 If you have a look at it at http://source.winehq.org/patches/ you see that the lines are wrapped. -- Best Regards, André Hentschel

1 0

inconsistencies in MCI headers w.r.t. to 64bit
by Joerg-Cyril.Hoehle＠t-systems.com 25 Mar '10

25 Mar '10

Dmitry, On 1/1/2004 you changed include/digitalv.h: DWORD dwReturn to DWORD_PTR However mmsystem.h uses DWORD. This is indeed what MSDN documents http://msdn.microsoft.com/en-us/library/dd798406%28VS.85%29.aspx http://msdn.microsoft.com/en-us/library/dd743409%28VS.85%29.aspx yet I refuse to believe it. I believe that all the MCI_XYZ_PARMS structures are consistent, so that a generic MCI string command parser is possible. That requires regular types. Eric Pouech wrote: http://www.winehq.org/pipermail/wine-devel/2010-February/081660.html >in that case, we (likely) need to write integral values as DWORD (on >both 32 and 64 bit systems) and strings & pointers as DWORD_PTR Indeed I believe that MCI_INTEGER (and hence dwReturn) should be mapped to DWORD and MCI_STRING (hence lpstrReturn) to DWORD_PTR. I believe EPP and part of AJ's recent winmm patches are wrong about 64bit. That could explain crashes or timeouts test.winehq shows for all machines running 64bit Wine (as opposed to 64bit Linux systems running 32bit Wine) in the mci tests. What evidence did you have for these changes? MSDN only or tests with natively compiled programs on native 64bit systems? Michael Stefaniuc wrote: http://www.winehq.org/pipermail/wine-devel/2010-February/081853.html >WTB uses Mingw to build the tests I could write a patch, but how to test it? WTB has 64bit boxes, but I don't know what headers WTB uses for compilation (MingW or Wine ones?) and thus whether to trust binaries compiled on WTB. OTOH, it might be that MS is really so messy that incompatible structure definitions are used and that the MCI command string parser is full of special fixes for particular devices. This might not be the first time. I already spotted a case with AVIVIDEO where the resource and structure layout are not in the same order: Look up SETVIDEO Quality and Algorithm and see for yourself. This means that Wine's regular parser -- and perhaps native as well, who knows -- fill the structure in an order incompatible with MCI_DGV_SETVIDEO_PARMS. It is possible that native's parser knows about this and fixes the order. As I don't know how to test that function, I never publicly wrote about this before. In the end, - what headers does WTB's MingW use for compilation? - Does anyone have an other idea why the mci and midi tests hang or crash on systems running wine64? Regards, Jörg Höhle

4 4