Re: gdiplus: Add a test to show that GdipCloneImage is not supposed to increase refcount of the source image stream, but clone it instead.
Vincent Povirk wrote: > I think it's most likely not keeping the input stream in the new image at all. > > It's probably also worth testing SelectActiveFrame, GetPropertyItem, > and GetImageRawFormat with clones of images. Maybe they don't work. A quick test shows that SelectActiveFrame called on a clone of a multiframe gif image succeeds under nt4 and win2k, but fails under all later gdiplus implementations. Looks like a clone does not really clone anything. -- Dmitry.
Re: gdiplus: Add a test to show that GdipCloneImage is not supposed to increase refcount of the source image stream, but clone it instead.
I think it's most likely not keeping the input stream in the new image at all. It's probably also worth testing SelectActiveFrame, GetPropertyItem, and GetImageRawFormat with clones of images. Maybe they don't work.
Re: include: fix mingw64 build
2012/6/29 Austin English : > On Thu, Jun 28, 2012 at 6:30 PM, Nicolas Le Cam wrote: >> 2012/6/29 Austin English : >>> Fixes http://bugs.winehq.org/show_bug.cgi?id=30980 >>> >>> -- >>> -Austin >>> >>> >>> >> Hi Austin, >> >> I already tried to fix it on Wine side, see [1], >> but Alexandre told me on IRC that the bug needs to be fixed in mingw-w64, >> as it shouldn't include crtdefs.h by default. >> >> BTW, won't that break mingw32 build ? >> >> [1] http://source.winehq.org/patches/data/84070 >> >> -- >> Nicolas Le Cam > > Thanks for the info. Have you or anyone else discussed this with > mingw64? I did a quick search on their bugtracker, but don't see > anything.. > http://sourceforge.net/search/?group_artifact_id=983354&type_of_search=artifact&group_id=202880&words=crtdefs > > -- > -Austin No I didn't take the time to report the issue. I cloned their repo and wanted to send a patch, unfortunately I didn't have the time to work on that actually. -- Nicolas Le Cam
Re: include: fix mingw64 build
On Thu, Jun 28, 2012 at 6:30 PM, Nicolas Le Cam wrote: > 2012/6/29 Austin English : >> Fixes http://bugs.winehq.org/show_bug.cgi?id=30980 >> >> -- >> -Austin >> >> >> > Hi Austin, > > I already tried to fix it on Wine side, see [1], > but Alexandre told me on IRC that the bug needs to be fixed in mingw-w64, > as it shouldn't include crtdefs.h by default. > > BTW, won't that break mingw32 build ? > > [1] http://source.winehq.org/patches/data/84070 > > -- > Nicolas Le Cam Thanks for the info. Have you or anyone else discussed this with mingw64? I did a quick search on their bugtracker, but don't see anything.. http://sourceforge.net/search/?group_artifact_id=983354&type_of_search=artifact&group_id=202880&words=crtdefs -- -Austin
Re: include: fix mingw64 build
2012/6/29 Austin English : > Fixes http://bugs.winehq.org/show_bug.cgi?id=30980 > > -- > -Austin > > > Hi Austin, I already tried to fix it on Wine side, see [1], but Alexandre told me on IRC that the bug needs to be fixed in mingw-w64, as it shouldn't include crtdefs.h by default. BTW, won't that break mingw32 build ? [1] http://source.winehq.org/patches/data/84070 -- Nicolas Le Cam
Re: wbemprox: Add a Win32_LogicalDisk class stub.
On Thu, 2012-06-28 at 22:22 +0300, John Yani wrote: > On 28 June 2012 22:17, Hans Leidekker wrote: > > On Thu, 2012-06-28 at 21:53 +0300, John Yani wrote: > > That's better. Does NFS actually query the Caption and Description > > properties? > > > > > Well, no. I just thought the output should be similar to the one on > winetestbot: https://testbot.winehq.org/JobDetails.pl?Key=19642&log_201=1#k201 No, that's just a test I wrote to investigate this stuff.
Re: wbemprox: Add a Win32_LogicalDisk class stub.
On Thu, 2012-06-28 at 21:53 +0300, John Yani wrote:
> +static void fill_logicaldisk( struct table *table )
> +{
> +static const WCHAR caption[] =
> +{'C',':',0};
> +static const WCHAR description[] =
> +{'L','o','c','a','l',' ','F','i','x','e','d',' ','D','i','s','k',0};
> +static const WCHAR device_id[] =
> +{'C',':',0};
> +struct record_logicaldisk *rec;
> +FIXME("returns a fake logical disks table\n");
> +if (!(table->data = heap_alloc( sizeof(*rec) )))
> +{
> +return;
> +}
> +rec = (struct record_logicaldisk *)(table->data);
> +rec->caption= caption;
> +rec->description= description;
> +rec->device_id = device_id;
> +table->num_rows = 1;
> +TRACE("created %u row(s)\n", table->num_rows);
That's better. Does NFS actually query the Caption and Description properties?
Re: user32: Add a test for ComboBox WM_WINDOWPOSCHANGED handling.
Sergey Guralnik writes: > @@ -5807,6 +5830,25 @@ static void test_combobox_messages(void) > log_all_parent_messages--; > ok_sequence(WmKeyDownComboSeq, "WM_KEYDOWN/VK_DOWN on a ComboBox", > FALSE); > > +wp.hwnd = combo; > +wp.hwndInsertAfter = HWND_TOP; > +wp.cx = wp.cy = wp.x = wp.y = 50; > +wp.flags = 0; > +flush_sequence(); > +SendMessage(combo, WM_WINDOWPOSCHANGED, 0, (LPARAM)&wp); It doesn't make much sense to send this message directly. What are you trying to test? -- Alexandre Julliard julli...@winehq.org
Re: [2/2] gdiplus: Increase refcount of the source stream when loading a bitmap instead of cloning it.
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.)
Re: Unused msvcp60 functions
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.
Re: Unused msvcp60 functions
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 http://fgouget.free.fr/ There are 10 types of people in the world... those who understand binary and those who don't.
Re: windowscodecs: Make sure that stream is not reused once the decoder is initialized.
Looks good to me.
mshtml: Always set the URL policy in the error cases?
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
Re: kernel32/tests: Remove an unused variable in the sync tests.
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
Re: include/basetsd.h: Fix int64 to int truncation warnings when compiling with a 64-bit PSDK compiler.
On 06/28/2012 01:50 PM, Dmitry Timoshkov wrote: > Michael Stefaniuc 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-truncation-from-double-to-float 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
Re: [4/5] d3dx9: Implement D3DXCreateVolumeTextureFromFileInMemoryEx.
2012/6/27 Józef Kucia :
> ---
> + 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.
Re: [1/5] d3dx9: Implement D3DXLoadVolumeFromMemory.
2012/6/27 Józef Kucia :
> ---
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?
Re: include/basetsd.h: Fix int64 to int truncation warnings when compiling with a 64-bit PSDK compiler.
Michael Stefaniuc 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-truncation-from-double-to-float > > 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.
Re: include/basetsd.h: Fix int64 to int truncation warnings when compiling with a 64-bit PSDK compiler.
On 06/28/2012 01:00 PM, Dmitry Timoshkov wrote: > Michael Stefaniuc wrote: > >> afair the fix is to disable the int truncation warnings in MSVC. For >> whatever reason they seem to enable those bogus warnings. > > The warnings are not bogus, the PSDK compiler also emits a warning when They are bogus as the C standard explicitly allows conversion between integer types. > 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. > 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. > patch also helps to pay more attention to warnings: when there are lots > of existing ones, it's hard to recognize new and possibly real problems > in the code. bye michael
Re: include/basetsd.h: Fix int64 to int truncation warnings when compiling with a 64-bit PSDK compiler.
Michael Stefaniuc wrote: > afair the fix is to disable the int truncation warnings in MSVC. For > whatever reason they seem to enable those bogus warnings. The warnings are not bogus, the PSDK compiler also emits a warning when truncating from double to float while gcc keeps silence for instance. Anyway, the patch shouldn't hurt, PSDK headers have similar casts. This patch also helps to pay more attention to warnings: when there are lots of existing ones, it's hard to recognize new and possibly real problems in the code. -- Dmitry.
Re: include/basetsd.h: Fix int64 to int truncation warnings when compiling with a 64-bit PSDK compiler.
Dmitry,
afair the fix is to disable the int truncation warnings in MSVC. For
whatever reason they seem to enable those bogus warnings.
bye
michael
On 06/28/2012 12:12 PM, Dmitry Timoshkov wrote:
> ---
> include/basetsd.h | 16
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/include/basetsd.h b/include/basetsd.h
> index 698dd5c..4a59897 100644
> --- a/include/basetsd.h
> +++ b/include/basetsd.h
> @@ -153,12 +153,12 @@ typedef unsigned int UHALF_PTR, *PUHALF_PTR;
>
> static inline ULONG32 HandleToULong(const void *h)
> {
> -return (ULONG_PTR)h;
> +return (ULONG32)(ULONG_PTR)h;
> }
>
> static inline LONG32 HandleToLong(const void *h)
> {
> -return (LONG_PTR)h;
> +return (LONG32)(LONG_PTR)h;
> }
>
> static inline void *ULongToHandle(ULONG32 ul)
> @@ -173,32 +173,32 @@ static inline void *LongToHandle(LONG32 l)
>
> static inline ULONG32 PtrToUlong(const void *p)
> {
> -return (ULONG_PTR)p;
> +return (ULONG32)(ULONG_PTR)p;
> }
>
> static inline LONG32 PtrToLong(const void *p)
> {
> -return (LONG_PTR)p;
> +return (LONG32)(LONG_PTR)p;
> }
>
> static inline UINT32 PtrToUint(const void *p)
> {
> -return (UINT_PTR)p;
> +return (UINT32)(UINT_PTR)p;
> }
>
> static inline INT32 PtrToInt(const void *p)
> {
> -return (INT_PTR)p;
> +return (INT32)(INT_PTR)p;
> }
>
> static inline UINT16 PtrToUshort(const void *p)
> {
> -return (ULONG_PTR)p;
> +return (UINT16)(ULONG_PTR)p;
> }
>
> static inline INT16 PtrToShort(const void *p)
> {
> -return (LONG_PTR)p;
> +return (INT16)(LONG_PTR)p;
> }
>
> static inline void *IntToPtr(INT32 i)
Re: [2/2] msvcp90: Sync spec files
On 28/06/2012 11:33, Owen Rudge wrote: --- dlls/msvcp60/msvcp60.spec | 638 ++-- Sorry, I ran the make_specfiles script without realising that msvcp60.spec shouldn't have been updated. I've resent the patch without these changes. -- Owen Rudge http://www.owenrudge.net/
Re: [2/2] msvcp90: Sync spec files
First patch looks correct. On 06/28/12 12:33, Owen Rudge wrote: dlls/msvcp60/msvcp60.spec | 638 ++-- You can't update msvcp60.spec file this way (most of functions there should not be forwarded to msvcp90).
Re: Unused msvcp60 functions
On 06/28/12 09:30, Francois Gouget wrote: I get the following warnings when compiling msvcp60: main.c:108:51: warning: ‘std_BADOFF_func’ defined but not used [-Wunused-function] misc.c:66:40: warning: ‘mutex_mutex_lock’ defined but not used [-Wunused-function] misc.c:73:40: warning: ‘mutex_mutex_unlock’ defined but not used [-Wunused-function] misc.c:80:40: warning: ‘mutex_mutex_ctor’ defined but not used [-Wunused-function] misc.c:87:40: warning: ‘mutex_mutex_dtor’ defined but not used [-Wunused-function] misc.c:175:58: warning: ‘set_new_handler_reset’ defined but not used [-Wunused-function] misc.c:127:50: warning: ‘wctype’ defined but not used [-Wunused-function] 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. All of these functions can be removed.
Unused msvcp60 functions
I get the following warnings when compiling msvcp60: main.c:108:51: warning: ‘std_BADOFF_func’ defined but not used [-Wunused-function] misc.c:66:40: warning: ‘mutex_mutex_lock’ defined but not used [-Wunused-function] misc.c:73:40: warning: ‘mutex_mutex_unlock’ defined but not used [-Wunused-function] misc.c:80:40: warning: ‘mutex_mutex_ctor’ defined but not used [-Wunused-function] misc.c:87:40: warning: ‘mutex_mutex_dtor’ defined but not used [-Wunused-function] misc.c:175:58: warning: ‘set_new_handler_reset’ defined but not used [-Wunused-function] Indeed msvcp60.spec contains no _BADOFF, _Mutex or set_new_handler function. I don't see them in my 'database' of exported Windows APIs for msvcp60 but maybe the Windows dlls I built it from where not entirely up to date. Still it looks like these were added in later revisions and thus can be removed from the above files. misc.c:127:50: warning: ‘wctype’ defined but not used [-Wunused-function] This one is actually forwarded to msvcp90 according to msvcp60.spec, thus making this implementation redundant. So it should either be removed or msvcp60.spec should be modified. -- Francois Gouget http://fgouget.free.fr/ "Only wimps use tape backup: _real_ men just upload their important stuff on ftp, and let the rest of the world mirror it ;)" -- Linus Torvalds
