June 2012 - Wine-Devel - list.winehq.org

Re: [2/2] gdiplus: Increase refcount of the source stream when loading a bitmap instead of cloning it.

by Vincent Povirk

This looks good to me. (I didn't think it was necessary to comment, but this is an important fix and I was surprised it was still New.)

June 28, 2012

Re: Unused msvcp60 functions

by Piotr Caban

On 06/28/12 16:33, Francois Gouget wrote: > Actually there are some implementation differences between msvcp60's and > msvcp90's misc.c code. In particular for the init_lockit(), > free_lockit() and various _Lockit_ctor_*() functions. Are the two > implementations supposed to be identical? I forgot that functions in this file were changed. This implementations are not supposed to be identical. > There's no indication that the > two misc.c files are supposed to remain synchronized but if that's the > goal, one option would be to move the msvcp90-specific functions to > another file so that it remains easy to copy and/or diff these two > misc.c files. All files in msvcp60 and msvcp90 will be quite similar. So it's useful if diff is minimal/readable. But I don't think that splitting most of the files (so some files may be copied) is useful. > Also a lot of functions actually rely on forwarding to msvcp90. Should > this be used for the misc.c functions too? Only if functions are operating on identical object in msvcp60 and msvcp90. I was trying to forward functions when it was possible.

June 28, 2012

Re: Unused msvcp60 functions

by Francois Gouget

On Thu, 28 Jun 2012, Piotr Caban wrote: [...] > The function were not removed to make msvcp60 and msvcp90 more similar > (and probably because I didn't use -Wunused-function option). Thanks to > it one can copy misc.c file between the dlls when it's modified. Actually there are some implementation differences between msvcp60's and msvcp90's misc.c code. In particular for the init_lockit(), free_lockit() and various _Lockit_ctor_*() functions. Are the two implementations supposed to be identical? There's no indication that the two misc.c files are supposed to remain synchronized but if that's the goal, one option would be to move the msvcp90-specific functions to another file so that it remains easy to copy and/or diff these two misc.c files. Also a lot of functions actually rely on forwarding to msvcp90. Should this be used for the misc.c functions too? -- Francois Gouget <fgouget(a)free.fr> http://fgouget.free.fr/ There are 10 types of people in the world... those who understand binary and those who don't.

June 28, 2012

Re: windowscodecs: Make sure that stream is not reused once the decoder is initialized.

by Vincent Povirk

Looks good to me.

June 28, 2012

mshtml: Always set the URL policy in the error cases?

by Francois Gouget

gcc 4.7 is complaining that policy may be used uninitialized in InternetHostSecurityManager_QueryCustomPolicy() and indeed there are many error cases in confirm_safety_load() and confirm_safety() where the policy is not set. The patch belowe seems to plug all these holes but I don't know if it really makes sense. diff --git a/dlls/mshtml/secmgr.c b/dlls/mshtml/secmgr.c index ff349ac..3f64a42 100644 --- a/dlls/mshtml/secmgr.c +++ b/dlls/mshtml/secmgr.c @@ -104,10 +104,9 @@ static HRESULT confirm_safety_load(HTMLDocumentNode *This, struct CONFIRMSAFETY CATID init_catid = CATID_SafeForInitializing; hres = ICatInformation_IsClassOfCategories(This->catmgr, &cs->clsid, 1, &init_catid, 0, NULL); + *ret = hres == S_OK ? URLPOLICY_ALLOW : URLPOLICY_DISALLOW; if(FAILED(hres)) return hres; - - *ret = hres == S_OK ? URLPOLICY_ALLOW : URLPOLICY_DISALLOW; } return S_OK; @@ -157,13 +156,17 @@ static HRESULT confirm_safety(HTMLDocumentNode *This, const WCHAR *url, struct C if(!This->catmgr) { hres = CoCreateInstance(&CLSID_StdComponentCategoriesMgr, NULL, CLSCTX_INPROC_SERVER, &IID_ICatInformation, (void**)&This->catmgr); - if(FAILED(hres)) + if(FAILED(hres)) { + *ret = URLPOLICY_DISALLOW; return hres; + } } hres = ICatInformation_IsClassOfCategories(This->catmgr, &cs->clsid, 1, &scripting_catid, 0, NULL); - if(FAILED(hres)) + if(FAILED(hres)) { + *ret = URLPOLICY_DISALLOW; return hres; + } if(hres != S_OK) { *ret = URLPOLICY_DISALLOW; -- 1.7.10

June 28, 2012

Re: kernel32/tests: Remove an unused variable in the sync tests.

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=19638 Your paranoid android. === WINEBUILD (build) === Patch failed to apply

June 28, 2012

Re: include/basetsd.h: Fix int64 to int truncation warnings when compiling with a 64-bit PSDK compiler.

by Michael Stefaniuc

On 06/28/2012 01:50 PM, Dmitry Timoshkov wrote: > Michael Stefaniuc <mstefani(a)redhat.com> wrote: > >>> truncating from double to float while gcc keeps silence for instance. >> I never looked at that but I assume the same holds true as above. > > The following thread has some interesting details: > http://www.daniweb.com/software-development/c/threads/114052/warning-c4305-… WOW, that's pretty stupid. "0.1" has no type, it is a numeric literal. Type is inferred from the context and only if that isn't possible it will fall back to the intrinsic type. You can even say "int i = 0.1;" and that is correct C. gcc -Wall -Wextra does not complain about the examples in the thread you quoted as that is the correct C behavior. MSVC is a C++ compiler beaten into submission to do C. And C++ perverted C. bye michael

June 28, 2012

Re: [4/5] d3dx9: Implement D3DXCreateVolumeTextureFromFileInMemoryEx.

by Matteo Bruni

2012/6/27 Józef Kucia <joseph.kucia(a)gmail.com>: > --- > + for (mip_level = 0; mip_level < mip_levels; mip_level++) > + { > + hr = calculate_dds_surface_size(src_info, width, height, &src_pitch, &mip_level_size); > + if (FAILED(hr)) return hr; ... > + hr = D3DXLoadVolumeFromMemory(volume, palette, NULL, pixels, src_info->Format, > + src_pitch, mip_level_size, NULL, &box, filter, color_key); mip_level_size would be the source slice pitch, right? I'd just call it like that.

June 28, 2012

Re: [1/5] d3dx9: Implement D3DXLoadVolumeFromMemory.

by Matteo Bruni

2012/6/27 Józef Kucia <joseph.kucia(a)gmail.com>: > --- Hi Józef, > +void copy_simple_data(const BYTE *src, UINT srcpitch, SIZE src_size, const PixelFormatDesc *srcformat, > + BYTE *dest, UINT destpitch, SIZE dst_size, const PixelFormatDesc *destformat, D3DCOLOR colorkey) DECLSPEC_HIDDEN; > + You are not actually using this function in the patch. > + if (dst_box->Left > dst_box->Right || dst_box->Right > desc.Width) > + return D3DERR_INVALIDCALL; > + if (dst_box->Top > dst_box->Bottom || dst_box->Bottom > desc.Height) > + return D3DERR_INVALIDCALL; > + if (dst_box->Front > dst_box->Back || dst_box->Back > desc.Depth) > + return D3DERR_INVALIDCALL; > + > + dst_width = dst_box->Right - dst_box->Left; > + dst_height = dst_box->Bottom - dst_box->Top; > + dst_depth = dst_box->Back - dst_box->Front; > + if (!dst_width || !dst_height || !dst_depth) > + return D3DERR_INVALIDCALL; You can avoid the last check by comparing e.g. (dst_box->Left >= dst_box->Right) above, instead of using ">". > + if (src_box->Left & (src_format_desc->block_width - 1) > + || src_box->Top & (src_format_desc->block_height - 1) > + || (src_box->Right & (src_format_desc->block_width - 1) > + && src_width != desc.Width) > + || (src_box->Bottom & (src_format_desc->block_height - 1) > + && src_height != desc.Height)) > + { > + WARN("Source box (%u, %u, %u, %u) is misaligned\n", > + src_box->Left, src_box->Top, src_box->Right, src_box->Bottom); > + return D3DXERR_INVALIDDATA; > + } I'm not quite sure of the right and bottom misalignment checks. If the tests confirm that, of course that's okay. > + for (slice = 0; slice < src_depth; slice++) > + { > + src_addr = src_memory; > + src_addr += slice * src_slice_pitch; Shouldn't you take into account src_box->Front?

June 28, 2012

Re: include/basetsd.h: Fix int64 to int truncation warnings when compiling with a 64-bit PSDK compiler.

by Dmitry Timoshkov

Michael Stefaniuc <mstefani(a)redhat.com> wrote: > > truncating from double to float while gcc keeps silence for instance. > I never looked at that but I assume the same holds true as above. The following thread has some interesting details: http://www.daniweb.com/software-development/c/threads/114052/warning-c4305-… > > Anyway, the patch shouldn't hurt, PSDK headers have similar casts. This > In this specific case it doesn't. Start littering the code with casts to > get rid of all those warnings and it will majorly hurt. Unneeded casts > are *BAD*; worse then ignoring bogus warnings from a compiler. Obviosuly I'm not planning to add the casts everywhere, but the inline constructs which specifically targeting the pointer to integer conversions that imply truncation should compile without warnings IMHO. -- Dmitry.

June 28, 2012