Re: [PATCH 2/2] d3d11.idl: Added some missing declarations.
On 09/11/2013 02:11 PM, Henri Verbeet wrote:
On 11 September 2013 12:09, Jacek Caban ja...@codeweavers.com wrote:
+typedef enum D3D11_RESOURCE_MISC_FLAG
+{
+D3D11_RESOURCE_MISC_GENERATE_MIPS= 0x0001,
+D3D11_RESOURCE_MISC_SHARED = 0x0002,
+D3D11_RESOURCE_MISC_TEXTURECUBE = 0x0004,
+D3D11_RESOURCE_MISC_DRAWINDIRECT_ARGS= 0x0010,
+D3D11_RESOURCE_MISC_BUFFER_ALLOW_RAW_VIEWS = 0x0020,
+D3D11_RESOURCE_MISC_BUFFER_STRUCTURED= 0x0040,
+D3D11_RESOURCE_MISC_RESOURCE_CLAMP = 0x0080,
+D3D11_RESOURCE_MISC_SHARED_KEYEDMUTEX= 0x0100,
+D3D11_RESOURCE_MISC_GDI_COMPATIBLE = 0x0200,
+D3D11_RESOURCE_MISC_SHARED_NTHANDLE = 0x0800,
+D3D11_RESOURCE_MISC_RESTRICTED_CONTENT = 0x1000,
+D3D11_RESOURCE_MISC_RESTRICT_SHARED_RESOURCE = 0x2000,
+D3D11_RESOURCE_MISC_RESTRICT_SHARED_RESOURCE_DRIVER = 0x4000,
+D3D11_RESOURCE_MISC_GUARDED = 0x8000
+} D3D11_RESOURCE_MISC_FLAG;
Arguably those should be __MSABI_LONG, although we may not care
enough. We certainly aren't very consistent about it in a lot of other
places either.
I just replaced the long numeric constants with the __MSABI_LONG.
But we don't need those anyway, it's mingw-w64 who needs them for C++
compatibility.
bye
michael
Re: Sequences in a loop
On 08/30/2013 02:33 PM, matyapiro31 wrote: I found many wine sources use not a pointer,but a sequence . for example: krnl386.exe16/dma.c: for(i=0,p=(char*)DMA_CurrentBaseAddress[channel];iret*size;i++) msdaps/usrmarshal.c: for(prop = 0; prop rgPropertySets[prop_set].cProperties; prop++) I know in some cases to use sequences are wiser way than pointers, but so many of them are used in loops. Should I change them? Optimizations should be primary for the human reader of the code. Compilers nowadays do a far better job at micro-optimization than humans. bye michael
Re: ntdll: Store all 'comClass' attributes (try3)
Hello Nikolay,
On 08/27/2013 12:16 PM, Nikolay Sivov wrote:
+static OLEMISC get_olemisc_value(const WCHAR *str, int len)
+{
+int min, max;
+
+min = 0;
+max = sizeof(olemisc_values)/sizeof(struct olemisc_entry) - 1;
+
+while (min = max)
+{
+int n, c;
+
+n = (min+max)/2;
+
+c = strncmpW(olemisc_values[n].name, str, len);
why don't you just compare the length first? If the length matches you
can do a simple strcmpW between the strings to verify the match.
Something like the below code:
if (olemisc_values[n].len == len)
{
if (!strcmpW(olemisc_values[n].name, str)
return olemisc_values[n].value;
else
...
}
else if (olemisc_values[n].len len)
max = n-1;
else
min = n+1;
+if (!c)
+{
+if (olemisc_values[n].len len)
+c = -1;
+else if (olemisc_values[n].len len)
+c = 1;
+}
+
+if (!c)
+return olemisc_values[n].value;
+
+if (c 0)
+max = n-1;
+else
+min = n+1;
+}
+
+WARN(unknown flag %s\n, debugstr_wn(str, len));
+return 0;
+}
bye
michael
Re: Wiki is down
On 08/23/2013 03:39 PM, Rosanne DiMesio wrote: I'm getting an Unable to connect message from Firefox and Connection closed by remote server from Opera. CC'ing Dimi as he might not read wine-devel so often. bye michael
Re: [website] Update git URL
On 08/16/2013 03:32 PM, Frédéric Delanoy wrote:
---
templates/en/cvs.template | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/templates/en/cvs.template b/templates/en/cvs.template
index 202cf60..300b9e4 100644
--- a/templates/en/cvs.template
+++ b/templates/en/cvs.template
@@ -3,9 +3,9 @@
h1 class=titleThe Wine CVS Tree/h1
Schouldn't the title change too? And nowadays people know what git is
and that's kinda the default SCM that people expect.
-pstrongNote: We have switched development from CVS to a
href=http://git.or.cz;Git/a.
+pstrongNote: We have switched development from CVS to a
href=http://git-scm.com/;Git/a.
Wine can no longer be downloaded via CVS.
- Instructions for using the a href={$root}/site/gitWine Git tree/a
+ Instructions for using the Wine Git tree
can be found on the a href=http://wiki.winehq.org/GitWine;GitWine/a
page on the a href=http://wiki.winehq.org;Wine Wiki/a./strong/p
bye
michael
Re: Sort the AUTHORS file by their last names.
On 08/16/2013 04:42 PM, Tae Wong wrote: Julliard said that it should feel free to provide a script that sorts by last name. You want to do this. Careful. It is a trap. He is fine with it if it is done *right*. Figuring out what the last name is is a hard problem. bye michael
Re: Sort the AUTHORS file by their last names.
On 08/16/2013 05:40 PM, Tae Wong wrote: In the GIT tree, the AUTHORS file is sorted on first names. Wrong. It is sorted by the text line. The first word on the line can be the first or lastname, depending what the author used. bye michael
Re: Sort the AUTHORS file by their last names.
On 08/16/2013 09:33 PM, Tae Wong wrote: You have to manually update it by adding all authors from the GIT log. The attachment is here at first message. That one you did is not correct either: - I see some Spanish double last names sorted by the 2nd one instead of the first. - In Dutch the van is not part of last name so you cannot sort those under V I'm sure there is more of that in other languages. Like I said this is not a trivial problem. And not a problem that can be automated thus not sustainable long term. There's a reason git opts to sort the text lines. bye michael
Re: Another major milestone
On 07/19/2013 01:40 PM, Marcus Meissner wrote: On Thu, Jul 18, 2013 at 04:55:42PM -0500, Jeremy White wrote: Alright folks, I have to confess that the 1.6 release came and I didn't immediately get up and dance. In fact, a new Wine release was almost...boring. I think we have to consider that a major milestone in of itself. New, useful releases are just a matter of course for us now. Woohoo! Now I'm dancing grin. I keep hearing Considering that it took you 15 years for 1.0 you are grinding them out really fast now ;) Soon he'll want us to do 6 or even 3 months time based releases! ;) bye michael
Fwd: Re: RFC: Three dots or Unicode ellipsis character
Original Message Subject: Re: RFC: Three dots or Unicode ellipsis character Date: Wed, 03 Jul 2013 19:32:15 +0200 From: Michael Stefaniuc mstef...@redhat.com To: Francois Gouget fgou...@free.fr On 07/03/2013 04:45 PM, Francois Gouget wrote: Wine uses three dots (...) rather than the Unicode ellipsis character (…) except in the Czech and Taiwanese translations that use the Unicode ellipsis character. I think we should be consistent but the question is which to use. I looked at the Windows interface guidelines and they don't have anything to say on this. http://msdn.microsoft.com/en-us/library/windows/desktop/aa974176.aspx#punctuation http://msdn.microsoft.com/en-us/library/windows/desktop/aa511453.aspx#ellipses Looking at what other platforms do, the Apple interface guidelines recommend using the Unicode ellipsis character saying assistive technologies need it. The GNOME and KDE guidelines don't have an official position but seem to use the Unicode character anyway: http://simos.info/blog/archives/17 So should we go with the flow and switch to using ellipsis characters or should we stick with three dots? How will the substitution work in the W2A case? bye michael
Re: winex11.drv: When skipping unknown attribute also skip it's value.
On 06/30/2013 11:29 PM, Ričardas Barkauskas wrote: --- dlls/winex11.drv/opengl.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 0001-winex11.drv-When-skipping-unknown-attribute-also-skip-.txt diff --git a/dlls/winex11.drv/opengl.c b/dlls/winex11.drv/opengl.c index 0f937cb..c7f78c5 100644 --- a/dlls/winex11.drv/opengl.c +++ b/dlls/winex11.drv/opengl.c @@ -921,7 +921,7 @@ static int ConvertAttribWGLtoGLX(const int* iWGLAttr, int* oGLXAttr, struct wgl_ TRACE(pAttr[%d] = GLX_RGBA_UNSIGNED_FLOAT_TYPE_EXT: %x\n, cur, pop); break ; default: - FIXME(unsupported %x WGL Attribute\n, iWGLAttr[cur]); + FIXME(unsupported %x WGL Attribute\n, iWGLAttr[cur++]); Side effects in the debug output functions should be avoided. break; } ++cur; bye michael
Re: widl: Do not generate C structure for empty interfaces.
On 06/26/2013 10:06 PM, Thomas Faber wrote: On 2013-06-26 21:47, André Hentschel wrote: The whitespace changes are bad, please don't do that. Ok, I can take them out no problem. Why's it bad though? I thought the rule was fix it while you're working on it anyway. Sure, fix the lines you modify and the block you're in. Avoid having whitespace only diff chunks. bye michael
Re: testbot and NT4
On 06/03/2013 10:36 PM, André Hentschel wrote: On 03.06.2013 22:22, Alexandre Julliard wrote: André Hentschel n...@dawncrow.de writes: I haven't heard about NT4 been taking out of the requirement. I guess the NT4 machines are offline due to the transition to the new testbot (newtestbot.winehq.org). Hopefully the machines are working again on one of the testbots soon, let's wait a bit. With the upcoming code freeze i'd say we need them. maybe someone else can tell more. I don't think we are going to bother setting up NT4 on the new testbot, it's no longer worth the trouble. Ah, ok. Didn't know. So the question is if François will continue to run winetest on his NT4 machines (outside of testbot), if so, we should not yet remove NT4 hacks in the testsuite. My guess is that the same rule applies as for the Win9x test hacks: Only remove when the culprit code is changed anyway. bye michael
Re: wineconsole screen scenarios
On 05/16/2013 03:34 PM, Hugh McMaster wrote: On Thursday, 16 May 2013 Rosanne DiMesio wrote: On Thu, 16 May 2013 08:04:07 -0400 Hugh McMaster wrote: My concern is with scenario (3). Wine is designed to be used with an X server, but wineconsole can be used in non-X-based environment. While this is possible, it would seem unlikely. Nonetheless, the issue has come up on the wine-devel list before. It's not common, but we have had users on the forum who are using wineconsole to run DOS apps. They're usually pretty adament about not wanting or needing X. Is it safe to assume then, that such users do not have any X libraries installed? Or do they simply choose not to use an X server? Yes, that is a safe assumption. Wine is *not* X specific anymore. On MacOSX you can use the native quartz driver. And I wouldn't wonder if Alexandre plans for a native driver for Android too. bye michael
Re: [PATCH 1/2] gdi32: Clip font glyphs to fit within text metrics. (try 2)
On 04/26/2013 05:54 AM, Sam Edwards wrote: On 04/25/2013 12:28 PM, Alexandre Julliard wrote: It doesn't work here: ../../../../wine/tools/runtest -q -P wine -M comctl32.dll -T ../../.. -p comctl32_test.exe.so ../../../../wine/dlls/comctl32/tests/listview.c touch listview.ok wine: Unhandled page fault on write access to 0x400053b34 at address 0x2b4c3d69e6b7 (thread 0398), starting debugger... winedbg: Internal crash at 0x2b53f78066b7 make: *** [listview.ok] Error 84 Thanks for the heads-up, AJ. I can't reproduce this problem, and it's strange that the debugger itself suffered an internal crash as well. Can anyone else verify? Did something mess up during AJ's build? Sam, look at the size of the address. It is the 64bit build that crashes. bye michael
Re: Clang static analyzer results / wine-1.5.28-66-g6899279
On 04/19/2013 12:43 AM, Austin English wrote: With wine-1.5.20 and clang 3.2, the test suite is in the same state on my Fedora 18 machine as gcc-4.7.2 llvm version: git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@179768 91177308-0d34-0410-b5e6-96231b3b80d8 clang version: git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@179771 91177308-0d34-0410-b5e6-96231b3b80d8 ./configure --disable-tests was used to reduce noise Static analyse of the tests is important as well. Over the years I found bad tests that didn't test what they were supposed to test. And even cases where the Wine code was bad due to that as it passed the tests. bye michael
wiki.winehq.org is down
Hey Dimi, the wiki is down: puk is the wiki down? puk it pings puk but with high latency puk and while port 80 connects a GET / doesn't returns thanks bye michael
Re: ntdll: make NtDelayExecution a bit more efficient
On 03/06/2013 07:10 PM, Graham wrote: Alexandre wrote: That's what the existing code already does. Indeed. I don't know what I was thinking... 2. If you're about to block on select(), then I don't see any point in preceding that with a call to sched_yield(). This was added for a reason; most likely you'll have to write tests. i.e. commit 8099c2b9. JW says ... to more closely resemble Windows behavior. The key is to yield in a Sleep... JW is Jeremy White so us old timers chuckle now ;) I haven't yet figured out how he came up with this analysis, but I think it's safe to assume that he is correct, and my patch is garbage. Lesson learned: consult history. Date: Tue Nov 2 19:32:03 2004 + and the comment mentions Win2K. If the ancient wisdom isn't backed by tests there's a fair chance that it might not be applicable today. Or that it was a wrong theory back then too. bye michael
Re: mmdevapi: Prevent 64 bit overflow within a few days of audio device use. (try 2)
Hello Joerg,
On 03/01/2013 10:22 AM, joerg-cyril.hoe...@t-systems.com wrote:
The idea to replace X * numerator / denominator
by X / den * mul + remainder from euclidian division
came from
http://blog.airsource.co.uk/index.php/2010/03/15/quelle-heure-est-il/
M. Stefaniuc suggested an inline function. The MulDiv64 inline has the benefit
of ensuring unsigned arithmetic, matching GetPosition UINT64 output.
My idea was to have the whole if else in an inline function.
But I see that even the whole functionality can move to a helper.
Something like this:
static inline ULONGLONG get_qpctime(void)
{
LARGE_INTEGER stamp, freq;
QueryPerformanceCounter(stamp);
QueryPerformanceFrequency(freq);
if(freq.QuadPart == 1000)
return stamp.QuadPart;
else
return stamp.QuadPart / freq.QuadPart * 1000
+ stamp.QuadPart % freq.QuadPart * 1000 /
freq.QuadPart;
}
bye
michael
Re: mmdevapi: Prevent 64 bit overflow within a few days of audio device use.
Hello Joerg, On 02/27/2013 02:46 PM, joerg-cyril.hoe...@t-systems.com wrote: The idea to replace X * numerator / denominator by X / den * mul + remainder from euclidian division came from can you please use an inline function for that? http://blog.airsource.co.uk/index.php/2010/03/15/quelle-heure-est-il/ thanks bye michael
Re: iphlpapi: Let C look like C.
Thanks Juan, On 02/07/2013 12:54 AM, Juan Lang wrote: Hi Michael, this isn't actually a problem with your patch, just something I spotted: On Wed, Feb 6, 2013 at 3:15 PM, Michael Stefaniuc mstef...@redhat.com mailto:mstef...@redhat.com wrote: On 02/06/2013 11:16 PM, Austin English wrote: On Feb 6, 2013 11:13 PM, Michael Stefaniuc mstef...@redhat.de mailto:mstef...@redhat.de mailto:mstef...@redhat.de mailto:mstef...@redhat.de wrote: --- dlls/iphlpapi/ipstats.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/dlls/iphlpapi/ipstats.c b/dlls/iphlpapi/ipstats.c index f8191ba..966d24e 100644 --- a/dlls/iphlpapi/ipstats.c +++ b/dlls/iphlpapi/ipstats.c @@ -693,7 +693,7 @@ DWORD WINAPI GetIcmpStatisticsEx(PMIB_ICMP_EX stats, DWORD family) } ret = GetIcmpStatistics(ipv4stats); -if SUCCEEDED(ret) +if (SUCCEEDED(ret)) This code is incorrect. All paths that set ret yield a non-negative value for ret, so SUCCEEDED(ret) is always 1. Using SUCCEEDED/FAILED on something other than an HRESULT is asking for trouble. The correct thing to do is if (!ret). can you please submit the patch so you get the commit glory? thanks bye michael
Re: iphlpapi: Let C look like C.
On 02/06/2013 11:16 PM, Austin English wrote:
On Feb 6, 2013 11:13 PM, Michael Stefaniuc mstef...@redhat.de
mailto:mstef...@redhat.de wrote:
---
dlls/iphlpapi/ipstats.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/dlls/iphlpapi/ipstats.c b/dlls/iphlpapi/ipstats.c
index f8191ba..966d24e 100644
--- a/dlls/iphlpapi/ipstats.c
+++ b/dlls/iphlpapi/ipstats.c
@@ -693,7 +693,7 @@ DWORD WINAPI GetIcmpStatisticsEx(PMIB_ICMP_EX
stats, DWORD family)
}
ret = GetIcmpStatistics(ipv4stats);
-if SUCCEEDED(ret)
+if (SUCCEEDED(ret))
{
stats-icmpInStats.dwMsgs = ipv4stats.stats.icmpInStats.dwMsgs;
stats-icmpInStats.dwErrors =
ipv4stats.stats.icmpInStats.dwErrors;
--
1.7.6.5
Minor nitpick, Make C look like C would be a more proper title, imho.
Yeah, but then I would describe what I'm doing and that is a big NO NO
as it is obvious from the patch. The intend is to let the Wine C code
feel like a first class C citizen ;-P
bye
michael
Re: ws2_32: Use assignment instead of memcpy to copy structs.
Forgot to add the credit for this patch series as I didn't wrote this
coccinelle script:
Found using the memcpy-assign.cocci script submitted for inclusion into
the Linux kernel:
https://systeme.lip6.fr/pipermail/cocci/2013-January/000121.html
bye
michael
On 01/24/2013 02:21 PM, Michael Stefaniuc wrote:
---
dlls/ws2_32/socket.c |2 +-
dlls/ws2_32/tests/sock.c |2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c
index c390828..6fd1e34 100644
--- a/dlls/ws2_32/socket.c
+++ b/dlls/ws2_32/socket.c
@@ -2183,7 +2183,7 @@ static BOOL interface_bind( SOCKET s, int fd, struct
sockaddr *addr )
if (setsockopt(fd, IPPROTO_IP, IP_UNICAST_IF, ifindex,
sizeof(ifindex)) != 0)
goto cleanup; /* Failed to suggest egress interface */
-memcpy(specific_interface_filter, generic_interface_filter,
sizeof(generic_interface_filter));
+specific_interface_filter = generic_interface_filter;
specific_interface_filter.iface_rule.k = adapter-Index;
filter_prog.len = sizeof(generic_interface_filter)/sizeof(struct
sock_filter);
filter_prog.filter = (struct sock_filter *)
specific_interface_filter;
diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c
index 56a64bc..4b4b99a 100644
--- a/dlls/ws2_32/tests/sock.c
+++ b/dlls/ws2_32/tests/sock.c
@@ -2725,7 +2725,7 @@ static void test_getsockname(void)
return;
}
-memcpy(sa_get, sa_set, sizeof(sa_set));
+sa_get = sa_set;
if (getsockname(sock, (struct sockaddr*) sa_get, sa_get_len) == 0)
ok(0, getsockname on unbound socket should fail\n);
else {
Re: [PATCH 1/3] include: Define FIELD_OFFSET to the standard offsetof macro
On 12/11/2012 10:20 AM, Dmitry Timoshkov wrote: Michael Stefaniuc mstef...@redhat.com wrote: On 12/10/2012 07:37 PM, Amine Khaldi wrote: This prevents the undefined behavior (null pointer dereference) diagnostics (clang with ubsan checks for example). This is a bug in clang. There is no null pointer dereference. Afair gcc tried to pull this trick too but got educated about their error. What's an advantage of these patches except of catering broken compilers/ checkers? I figure you mean the 2nd try which uses offsetof unconditionally. The advantage is that it replaces the cast orgy with the C89 standard construct thus it easier to read. Appeasing the still wrong clang check is just a side effect. bye michael
Re: [PATCH 1/3] include: Define FIELD_OFFSET to the standard offsetof macro
On 12/10/2012 07:37 PM, Amine Khaldi wrote: This prevents the undefined behavior (null pointer dereference) diagnostics (clang with ubsan checks for example). This is a bug in clang. There is no null pointer dereference. Afair gcc tried to pull this trick too but got educated about their error. --- include/winnt.h | 4 1 file changed, 4 insertions(+) diff --git a/include/winnt.h b/include/winnt.h index 207adaa..a3b996a 100644 --- a/include/winnt.h +++ b/include/winnt.h @@ -746,8 +746,12 @@ typedef struct _MEMORY_BASIC_INFORMATION #define UNICODE_STRING_MAX_CHARS 32767 +#if defined(__GNUC__) || defined(__clang__) +#define FIELD_OFFSET(type, field) offsetof(type, field) +#else #define FIELD_OFFSET(type, field) \ ((LONG)(INT_PTR)(((type *)0)-field)) +#endif #define CONTAINING_RECORD(address, type, field) \ ((type *)((PCHAR)(address) - (PCHAR)(((type *)0)-field))) bye michael
Re: [PATCH] amstream: Initialize correctly AM_MEDIA_TYPE struct.
Hello Christian, On 11/15/2012 09:43 AM, Christian Costa wrote: Fixes bug 32185. --- dlls/amstream/mediastreamfilter.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/dlls/amstream/mediastreamfilter.c b/dlls/amstream/mediastreamfilter.c index 9b2bd8e..d5388e5 100644 --- a/dlls/amstream/mediastreamfilter.c +++ b/dlls/amstream/mediastreamfilter.c @@ -132,6 +132,8 @@ static HRESULT WINAPI BasePinImp_GetMediaType(BasePin *This, int index, AM_MEDIA MSPID purpose_id; int i; +ZeroMemory(amt, sizeof(*amt)); + this looks odd. There is no check if amt is NULL, at least that's what caught my attention. So I've looked around and stumbled upon the AM_MEDIA_TYPE structure documentation http://msdn.microsoft.com/en-us/library/windows/desktop/dd373477%28v=vs.85%29.aspx pUnk Not used. Set to NULL. Setting that to NULL in amstream's BasePinImp_GetMediaType() fixes the crash for me. But who's responsibility is to set / check that? That function or the caller aka test_media_streams() ? bye michael
Re: [PATCH] tools/widl/header.c: Add a way to declare interface data members.
On 11/11/2012 03:00 AM, Max TenEyck Woodbury wrote: I mentioned this a few days ago. It would have helped if you had raised this point then. I tried, but your email made no sense. You didn't even mention that you're talking about COM interfaces. As it stands, it is simply a way to adding data members to an aggregate with an interface. In that sense it is not an addition to the interface since the Vtbl pointer remains exactly as before. The new information is not part of the interface as such. Putting it in the same 'struct' as the Vtbl is simply a way of enforcing the association. If you do not define a iface_IFACE_DATA macro, nothing happens. bye michael
Re: [PATCH] tools/widl/header.c: Add a way to declare interface data members.
On 11/11/2012 07:12 AM, Max TenEyck Woodbury wrote: On 11/11/2012 01:01 AM, Nikolay Sivov wrote: On 11/11/2012 05:00, Max TenEyck Woodbury wrote: I mentioned this a few days ago. It would have helped if you had raised this point then. As it stands, it is simply a way to adding data members to an aggregate with an interface. Data members to an aggregate? What are you talking about and what it has to do with interface definition? An aggregate is a collection of information, like a class, struct or union. Your mixing up terminology. Some aggregates include 'interface's, a COM, OLE or RPC thingy. The interface can have only methods defined, but those methods might want access to some additional data. To get to that data, the method now has to build a pointer to the containing aggregate and reference the data through that pointer. This introduces complications to the code since the data may not be in same place in the aggregate in each instance where the interface is used. You need a slightly different code sequence for each different place the method is needed. However, the source code will be virtually identical for each instance. This patch allows those aggregate data members associated with the interface, which are not technically part of the interface, to be placed in a fixed relation to the interfaces Vtbl pointer. (Practically the Vtbl pointer is the interface.) By establishing such a relationship, the need to convert from the pointer to the interface (specifically to its Vtbl pointer) to a pointer to its containing aggregate in order to get to the relevant data is removed. Please check how a C compiler is laying out structs in memory. In both cases the data is at a fixed offset (calculated at compile time) in relation to the interface. Now, technically, the associated data is not part of the interface. It is part of the aggregate containing and implementing the interface. Moving the declaration from the aggregate to the end of the struc for the interface is a hack that lets simpler and more general code to be generated. So, it's a hack, that you only use if you want to, to speed up execution and simplify maintenance. It is a hack that breaks the ABI (sizeof(interface) == sizeof(void*)), doesn't improves the generated code, doesn't simplify maintenance at all, quite the contrary. This sounds more like trolling than a serious attempt to improve Wine. bye michael
Re: [PATCH] tools/widl/header.c: Add a way to declare interface data members.
You can't do that as the COM standard specifies also the ABI.
A COM interface is a pointer to a virtual table; it is *not* a struct.
That's just an implementation detail in C.
On 11/10/2012 08:48 PM, m...@mtew.isa-geek.net wrote:
From: Max TenEyck Woodbury max+...@mtew.isa-geek.net
---
tools/widl/header.c | 14 ++
1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/tools/widl/header.c b/tools/widl/header.c
index 2f275c7..965dcbc 100644
--- a/tools/widl/header.c
+++ b/tools/widl/header.c
@@ -1159,6 +1159,17 @@ static void write_com_interface_start(FILE *header,
const type_t *iface)
fprintf(header,#define __%s_%sINTERFACE_DEFINED__\n\n, iface-name,
dispinterface ? DISP : );
}
+static void write_interface_data_macro(FILE *header, const type_t *iface)
+{
+
+ if (type_iface_get_inherit(iface))
+write_interface_data_macro(header, type_iface_get_inherit(iface));
+
+ fprintf(header, #ifdef %s_IFACE_DATA\n, iface-name);
+ fprintf(header, %s_IFACE_DATA\n, iface-name);
+ fprintf(header, #endif /* defined %s */\n, iface-name);
+}
+
static void write_com_interface_end(FILE *header, type_t *iface)
{
int dispinterface = is_attr(iface-attrs, ATTR_DISPINTERFACE);
@@ -1214,6 +1225,9 @@ static void write_com_interface_end(FILE *header,
type_t *iface)
fprintf(header, } %sVtbl;\n, iface-name);
fprintf(header, interface %s {\n, iface-name);
fprintf(header, CONST_VTBL %sVtbl* lpVtbl;\n, iface-name);
+
+ write_interface_data_macro(header, iface);
+
fprintf(header, };\n);
fprintf(header, \n);
fprintf(header, #ifdef COBJMACROS\n);
bye
michael
Re: [PATCH 1/3] dmloader: COM cleanup of IDirectMusicLoader object.
On 11/08/2012 01:13 PM, Christian Costa wrote: 2012/11/8 Henri Verbeet hverb...@gmail.com mailto:hverb...@gmail.com On 8 November 2012 00:22, Michael Stefaniuc mstef...@redhat.com mailto:mstef...@redhat.com wrote: But using just the capitalized letters from the name of the COM class as a prefix and skipping the Impl would be in hindsight the better standard. There are still 170+ COM interfaces to clean up which is a sizable number regardless of it being just 13% of the total interface implementations, so we could still change the standard, especially as the existing function/method naming standard is not strictly enforced; I didn't bother changing offenders if the name was reasonable. But I'm deferring this decision to Jacek / Alexandre as they are the drivers of the COM standardization in Wine. I don't mind too much as I can work with both patterns. I think the only reasonable naming convention is to name things after the implementation structure. In this case that would still end up being IDirectMusicLoaderImpl_..., but for a slightly different reason. Where I agree with Nikolay is that dmloader would be a much nicer name than IDirectMusicLoaderImpl for the implementation structure as well, in which case you would also end up with dmloader_... for method implementations. dmloader_IDirectMusicLoader_Method or dmloader_Method? dmloader_IDirectMusicLoader_Method I was just saying removing the interface name was not a good thing imo or am I missing something? Right, the interface name needs to be there as it matches the COBJMACROS name. Basically the C macro with a prefix. bye michael
Re: [PATCH 1/3] dmloader: COM cleanup of IDirectMusicLoader object.
On 11/08/2012 02:50 PM, Nikolay Sivov wrote: On 11/8/2012 15:41, Michael Stefaniuc wrote: On 11/08/2012 01:13 PM, Christian Costa wrote: 2012/11/8 Henri Verbeet hverb...@gmail.com mailto:hverb...@gmail.com On 8 November 2012 00:22, Michael Stefaniuc mstef...@redhat.com mailto:mstef...@redhat.com wrote: But using just the capitalized letters from the name of the COM class as a prefix and skipping the Impl would be in hindsight the better standard. There are still 170+ COM interfaces to clean up which is a sizable number regardless of it being just 13% of the total interface implementations, so we could still change the standard, especially as the existing function/method naming standard is not strictly enforced; I didn't bother changing offenders if the name was reasonable. But I'm deferring this decision to Jacek / Alexandre as they are the drivers of the COM standardization in Wine. I don't mind too much as I can work with both patterns. I think the only reasonable naming convention is to name things after the implementation structure. In this case that would still end up being IDirectMusicLoaderImpl_..., but for a slightly different reason. Where I agree with Nikolay is that dmloader would be a much nicer name than IDirectMusicLoaderImpl for the implementation structure as well, in which case you would also end up with dmloader_... for method implementations. dmloader_IDirectMusicLoader_Method or dmloader_Method? dmloader_IDirectMusicLoader_Method I don't think it's better than it is now. It is better as it doesn't contain the same ICamelCaseName twice. Of course the object should be renamed then to dmloader. This is a workable solution for the people that insist on having the object name in function implementing the method. I was just saying removing the interface name was not a good thing imo or am I missing something? Right, the interface name needs to be there as it matches the COBJMACROS name. Basically the C macro with a prefix. Why? If you really want to keep interface name the better way imho is as it's usually done in mshtml, like HTMLDOMTextNode_*, so here you don't need to add anything like prefix. The first rule of COM in Wine is: Do what Jacek says, not what he does! ;) HTMLDOMTextNode_* aka dropping the I is fine too as long as you do *not* use the same prefix for other functions. E.g. I've seen that style used for helpers. bye michael
Re: [PATCH 1/3] dmloader: COM cleanup of IDirectMusicLoader object.
On 11/08/2012 03:44 PM, Christian Costa wrote: 2012/11/8 Michael Stefaniuc mstef...@redhat.com mailto:mstef...@redhat.com On 11/08/2012 01:13 PM, Christian Costa wrote: 2012/11/8 Henri Verbeet hverb...@gmail.com mailto:hverb...@gmail.com mailto:hverb...@gmail.com mailto:hverb...@gmail.com On 8 November 2012 00:22, Michael Stefaniuc mstef...@redhat.com mailto:mstef...@redhat.com mailto:mstef...@redhat.com mailto:mstef...@redhat.com wrote: But using just the capitalized letters from the name of the COM class as a prefix and skipping the Impl would be in hindsight the better standard. There are still 170+ COM interfaces to clean up which is a sizable number regardless of it being just 13% of the total interface implementations, so we could still change the standard, especially as the existing function/method naming standard is not strictly enforced; I didn't bother changing offenders if the name was reasonable. But I'm deferring this decision to Jacek / Alexandre as they are the drivers of the COM standardization in Wine. I don't mind too much as I can work with both patterns. I think the only reasonable naming convention is to name things after the implementation structure. In this case that would still end up being IDirectMusicLoaderImpl_..., but for a slightly different reason. Where I agree with Nikolay is that dmloader would be a much nicer name than IDirectMusicLoaderImpl for the implementation structure as well, in which case you would also end up with dmloader_... for method implementations. dmloader_IDirectMusicLoader_Method or dmloader_Method? dmloader_IDirectMusicLoader_Method Henri said the other. It seems there is no consensus. ;) Of course there is consensus. The consensus is: - It depends on the situation - There are acceptable naming conventions - IDirectMusicLoaderImpl_IDirectMusicLoader_QueryInterface is not one of the acceptable solutions :) bye michael
Re: [PATCH 1/3] dmloader: COM cleanup of IDirectMusicLoader object.
On 11/07/2012 02:50 PM, Christian Costa wrote: 2012/11/7 Nikolay Sivov bungleh...@gmail.com mailto:bungleh...@gmail.com On 11/7/2012 01:05, Christian Costa wrote: Le 06/11/2012 22:38, Michael Stefaniuc a écrit : On 11/06/2012 08:51 PM, Christian Costa wrote: Le 06/11/2012 20:26, Nikolay Sivov a écrit : On 11/6/2012 20:47, Christian Costa wrote: What I also meant is these names are redundant, something like directmusicloader_AddRef() is enough, of course you could use mixed casing in names if you want, there's no strict guidelines for that afaik. I like to see the interface name in the function name because the method alone does not mean anything. Something like this: object nameImpl_interface name_method name but in some case where there is only 1 interface for the object: interface nameImpl_method name is better. The interface name should be clearly visible IMO it's easier to read in a log file. IDirectMusicLoaderImpl___IDirectMusicLoader_AddRef I disagree. It is redundant, too long and thus hard to read. I even have patches that remove some of them. I've never submitted those as you beat me to the dm* COM cleanup. I agree too if there is only one interface. I think it is but I would like to be sure before submitting a patch. I will check it and submit a patch for that. Too long and hard to read, I don't agree. See my response to Nikolay. Your example of having two methods with the same name in same object could be different things: - two separate not inherited interfaces will have IUnknown part in common at least, in this case it doesn't make sense to have traces from all of them, you use main interface to implement IUnknown part and forward to it. I just use an example to show that we can have methods with the same name. This other cases too off course. Having methods like maininterface_another interface_AddRef is strange when you can reduce that to another inerface_AddRef I didn't write this code and I don't like the current name either but a method name does not mean anything if you don't know the interface it belongs to. And I would put the real interface name as for the method. It's a bit like renaming GetBufferDataPointer method to getbuf just because it's shorter. Actually that's what Nikolay said. The function name needs to be interfaceImpl_method where interface/method are the official names from the API. What is superfluous/redundant information is to prepend all that with the object name. Which in the case of dm* is the name of the primary interface + Impl. - object implements interface that overrides some of methods from an interface it's inherited, this is a bit special and in dwrite for example it's done like dwritetextlayout_layout___GetFontCollection() and dwritetextlayout___GetFontCollection(). But that is more an exception because of dwrite C++ nature; The method name is GetFontCollection but what is the interface ? ILayout, ITextLayout, ... I have to dig in the code and do some grep. And why don't you use getfontcollection instead, or gfntcollec? Why it's good to have short and clean names? Because that will get you clean traces where name doesn't eat half of line width. It seems clean is somewhat subjective. I prefer also short line in log but this meaningfull name. Just take quartz with object that can have up to 10 interfaces and interfaces that can be implemented by up to 10 objects. Put a bit of multithreading on top of it (and I don't mention the fact the TRACE are not serialized between thread). I the case of quartz most of the duplicated interfaces are implemented by the same base (C++) object. Or should be implemented that way. So while multiple COM classes support the same interface most of the time there is one implementation only. If not the next question to ask is if it really makes sense to have two monster COM classes in the same .c file. Of course Alexandre might not be convinced about a file split but in that case to avoid functions with the same name use something short to prefix it. I prefer using the upper letters of the COM class as a prefix, e.g.: dsb_IUnknown_QueryInterface as opposed to IDirectSoundBuffer8Impl_IUnknown_QueryInterface. By the time I get to the method name I have forgotten the starting part of the function name. Also I don't get confused by seeing two interface names in the same function name
LPJUNK for COM Interfaces (Was: Re: [PATCH 1/3] dmloader: COM cleanup of IDirectMusicLoader object.)
On 11/06/2012 11:05 PM, Christian Costa wrote: Sure, separate patch to remove the duplicate name is fine. Just start with that as it makes the subsequent COM cleanup patch shorter as it cleans up the function header (whitespace and LPJUNK). That's what I try to do altough I don't clearly understand what the problem with LPJUNK stuff. For the normal LPJUNK there is no problem. Except the programmers that use it. Then you get beautiful code like: include/rpcasync.h:RPCRTAPI RPC_STATUS RPC_ENTRY RpcGetAuthorizationContextForClient(RPC_BINDING_HANDLE,BOOL,LPVOID,PLARGE_INTEGER,LUID,DWORD,PVOID,PVOID*); LPVOID, PVOID, PVOID* In the same line. If they would have used LPVOID, LPVOID, LPLPVOID I could have at least appreciate their effort to stay consistent. Of course there is no LPLPVOID... So for the normal LPJUNK inconsistency is the main issue and there is code out there that uses every possible permutations of saying void*. In the same function. Adding const into the mix makes it more interesting. But overall it is easy to decode: you tread a postfix * for a prefix LP: LPVOID = VOID* No surprises there even if you count in the two notable exceptions: LPSTR == char * LPWSTR == WCHAR * The LPJUNK for COM interface types has a far bigger problem, it looses information: LPDIRECTSOUNDBUFFER == DIRECTSOUNDBUFFER* ?? Add some LPDSCBUFFERDESC and similar to the same line of code and you lost track. I'm singling out dsound here but ddraw/d3d with their monster functions with 10+ parameters was way worse to figure out. Especially when the implementation was LPJUNK casting those parameters to pass them to other functions: Is there a object == interface cast hidden there? Or maybe a cast from one interface to the other where it should instead use QueryInterface? bye michael
Re: [PATCH 1/3] dmloader: COM cleanup of IDirectMusicLoader object.
On 11/07/2012 06:40 PM, Christian Costa wrote: 2012/11/7 Michael Stefaniuc mstef...@redhat.com mailto:mstef...@redhat.com On 11/07/2012 02:50 PM, Christian Costa wrote: I didn't write this code and I don't like the current name either but a method name does not mean anything if you don't know the interface it belongs to. And I would put the real interface name as for the method. It's a bit like renaming GetBufferDataPointer method to getbuf just because it's shorter. Actually that's what Nikolay said. The function name needs to be interfaceImpl_method where interface/method are the official names from the API. What is superfluous/redundant information is to prepend all that with the object name. Which in the case of dm* is the name of the primary interface + Impl. So in this particular case: IDirectMusicLoaderImpl_AddRef, right ? Right. - object implements interface that overrides some of methods from an interface it's inherited, this is a bit special and in dwrite for example it's done like dwritetextlayout_layout___GetFontCollection() and dwritetextlayout___GetFontCollection(). But that is more an exception because of dwrite C++ nature; The method name is GetFontCollection but what is the interface ? ILayout, ITextLayout, ... I have to dig in the code and do some grep. And why don't you use getfontcollection instead, or gfntcollec? Why it's good to have short and clean names? Because that will get you clean traces where name doesn't eat half of line width. It seems clean is somewhat subjective. I prefer also short line in log but this meaningfull name. Just take quartz with object that can have up to 10 interfaces and interfaces that can be implemented by up to 10 objects. Put a bit of multithreading on top of it (and I don't mention the fact the TRACE are not serialized between thread). I the case of quartz most of the duplicated interfaces are implemented by the same base (C++) object. Or should be implemented that way. So while multiple COM classes support the same interface most of the time there is one implementation only. It's partly true. Sometimes some methods are overridden. If not the next question to ask is if it really makes sense to have two monster COM classes in the same .c file. Yes of course but you will have the same function name in the debugger and in the trace. I'll pass on the debugger as I'm not a debugger person. TRACE on the other hand is simple. Sometimes there is a different debug channel in use. Or the trace message itself can contain the info. But I also made the experience that more often than not one has to follow the interface/object pointer from the TRACE to be able to distinguish between different instances of the same COM class. I mean follow it back to the creation/initialization of the object which gives the needed info to find the right implementation. For example in dmusic/port.c there is 3 port objects for Synth, MidiOut and MidiIn each one supported 3 interfaces IDirectMusicPort, IDirectMusicPassThru and possibly IDirectMusicPortDownload. Altough it is ok to use for example IDirectMusicPortImpl_Method for the common methods implementation but you will need to specify the object name for the methods that are specific to each port. Unless you keep a generic implementation and use I've seen different techniques in use, it really depends on how big the implementations differ. It might be even solvable with an if-else if (This-has_ds8) // if is_primary(This) foo; else bar; Other times there is a different vtable with different degrees of methods in common. a functions table as it is done sometimes in strmbase but is less C++ like. You make it sound like less C++ like would be bad. Quite the contrary, not being bound by the C++ object model gives you flexibility. There was a great LWN article on the object oriented programming/design patterns in the Linux Kernel, it is worth a read for anybody interested in object oriented programming in C. Of course Alexandre might not be convinced about a file split but in that case to avoid functions with the same name use something short to prefix it. I prefer using the upper letters of the COM class as a prefix, e.g.: dsb_IUnknown_QueryInterface as opposed to IDirectSoundBuffer8Impl_IUnknown_QueryInterface. By the time I get to the method name I have forgotten the starting part of the function name. Also I don't get confused by seeing two interface names in the same function name. Personally I would have used : dsb_IUnknown_QueryInterface dsb_IDirectSoundBuffer8_QueryInterface dsb_I_ So we have object_interface_method. This is more
Re: [PATCH 1/3] dmloader: COM cleanup of IDirectMusicLoader object.
On 11/06/2012 08:51 PM, Christian Costa wrote: Le 06/11/2012 20:26, Nikolay Sivov a écrit : On 11/6/2012 20:47, Christian Costa wrote: What I also meant is these names are redundant, something like directmusicloader_AddRef() is enough, of course you could use mixed casing in names if you want, there's no strict guidelines for that afaik. I like to see the interface name in the function name because the method alone does not mean anything. Something like this: object nameImpl_interface name_method name but in some case where there is only 1 interface for the object: interface nameImpl_method name is better. The interface name should be clearly visible IMO it's easier to read in a log file. IDirectMusicLoaderImpl_IDirectMusicLoader_AddRef I disagree. It is redundant, too long and thus hard to read. I even have patches that remove some of them. I've never submitted those as you beat me to the dm* COM cleanup. And for example if an object has 2 interfaces X and Y that both derives from Z then you will have 2 methods with the same name. How will you make the distinction ? You don't need to make a distinction as you have in most of the cases only one implementation. The stuff at the moment is highly duplicated. Needs either to use a solution like strmbase where a base object is included or COM aggregation. Didn't dig too deep into that as I stopped my COM cleanup work for dm* for now. If dmloader object has really 1 interface, I would rename then IDirectMusicLoaderImpl_AddRef. Although this is probably the case, I'm not sure yet. In all cases, I would leave this for another patch. One thing at once. Sure, separate patch to remove the duplicate name is fine. Just start with that as it makes the subsequent COM cleanup patch shorter as it cleans up the function header (whitespace and LPJUNK). bye michael
Re: [PATCH 1/2] user32: Remove redundant sizeof check.
On 10/24/2012 11:04 AM, Ken Thomases wrote: On Oct 24, 2012, at 3:24 AM, Michael Stefaniuc wrote: @@ -1330,9 +1330,6 @@ static HICON CURSORICON_LoadFromFile( LPCWSTR filename, } dir = (const CURSORICONFILEDIR*) bits; -if ( filesize sizeof(*dir) ) -goto end; - if ( filesize (sizeof(*dir) + sizeof(dir-idEntries[0])*(dir-idCount-1)) ) goto end; That doesn't seem redundant to me. It's not safe to access dir-idCount if the file isn't known to be big enough. Actually it is safe. The file is mapped in page size chunks. So accessing dir-idCount is safe aka no exception will be generated. And filesize will be smaller for any random WORD that ends up being in dir-idCount. bye michael
Re: [PATCH 5/6] dsound: rework ugly mixer logic
Hello Jörg, On 10/24/2012 12:36 PM, joerg-cyril.hoe...@t-systems.com wrote: I'm not happy with Wine's DSound+WinMM-mmdevapi. That may be a good move for MS, but it's a bad move for Wine. Wine's audio drivers don't what would you do instead? Go back to the old way and have dsound and winmm drivers too? That road leads to madness as it will triple the amount of sound drivers in Wine. bang the HW, they have huge SW stacks like PA or Jack in their back. bye michael
Re: d3dx9_36: Add tests for D3DXSHRotateZ
On 10/22/2012 11:21 AM, Henri Verbeet wrote: On 22 October 2012 09:06, Rico Schüller kgbric...@web.de wrote: On 19.10.2012 13:15, Nozomi Kodama wrote: This patch adds tests for the patch sent by Rico for D3DXSHRotateZ. Moreover, I changed const in CONST in the declaration of the function in order to uniformize with the file. +const FLOAT angle[] = ... I'd prefer const because it is used a little bit more often than CONST, but it is only a matter of taste. git grep [ (,]const | wc -l ~61k git grep [ (,]CONST | wc -l ~1.2k IIRC CONST is some kind of hack for compilers that don't (fully) support const. I think it's unlikely you'll be able to compile a working Wine with such a compiler even if you can find one, these days probably even things like MSVC, Sun Studio and Clang support const. And even if you could, something along the lines of AC_C_CONST would probably be a better solution anyway. It seems tempting to cleanup the couple of places in Wine that use CONST, and add a __ONLY_IN_WINELIB to the #define in include/windef.h. I'm removing CONST in the lines I touch so +1 on a concerted effort to eradicate it. Though at 1.2k lines Alexandre might consider it too spammy. bye michael
Re: [PATCH 10/25] mciseq: Limit concurrency when starting to play.
On 10/10/2012 10:42 AM, joerg-cyril.hoe...@t-systems.com wrote: Hi, David Laight wrote: Better to code as: status = wmm-dwStatus; if (...) Incidentally, that's what I did tonight in patch 20/25 try 2. but even then I think the compiler is allowed to perform extra reads. It might, for example, do so if the condition was complicayted and there was a lot of pressure on registers - so instead of spilling 'status' to stack, it would re-read it. Interesting. Do you think/believe or are you sure? With gcc at -O0 'status' will get a stack slot http://gcc.gnu.org/ml/gcc/2010-05/msg00116.html With optimization turned on probably not as it is just a reference to an other memory location which can be easily optimized away. With gcc you can force it to only to one read by adding: asm volatile(:::memory); The only portable way is with volatile. only portable way from the C perspective. However, given a specific target environment, we could use its API functions. Either MemoryBarrier(); /* which MSDN documents but Wine does not provide in include/*.h Or InterlockedExchange(status, wmm-dwStatus); So if AJ is still not satisfied with try 2, I'll change all reads of wmm-dwStatus within the player into InterlockedExchange. Yet I think that would be a superfluous extra memory barrier within the player. bye michael
Re: dsound: Don't bother shrinking the secondary buffer list.
Hello Jörg, as always it depends. On 09/27/2012 09:37 AM, joerg-cyril.hoe...@t-systems.com wrote: +#include assert.h your patch is already committed, yet I believe that the least that dsound needs is new asserts. Asserts in dsound have been a PITA in Wine for the last decade. It's ok for the kernel to crash in an assert. IMHO it is not ok for an optional thing like sound. The assert's backtrace will not help anybody, because the logical error will have happened much earlier. I disagree. It helps GyB to open regression bug reports and it helps me to fix them. Instead, a plain ERR would have been just as telling. Let's quote http://www.winehq.org/docs/winedev-guide/debugging#DBG-CLASSES ERR: Messages in this class indicate serious errors in Wine, such as as conditions that should never happen by design. I recommend: -assert(device-buffers[0] == pDSB); + if (pdevice-buffers[0] != pDSB) + ERR(broken device buffer %p\n, device-buffers[0]); HeapFree(GetProcessHeap(), 0, device-buffers); device-buffers = NULL; The ERR is IMHO revealing enough in a user's log. That's too late if it hits the user. I had reviewed the code: That code is an implementation detail completely outside the control of the applications. Barring a memory corruption there is no way for it to fail. The assert is there for two reasons: - To crash during make test when the next developer touches that code. - As a comment less likely to be removed by Alexandre ;) If a sound component detects a bogus state, I'd very much prefer that it disables sound -- without crashing. In general it depends of course. In this particular case the ERR would be useless noise as the assert is only for the last sound buffer that is removed; usually at program exit. Until then the code does what you prefer: silently drops sound buffers without crashing thus deferring the crash to the program exit. A crash on program exit does get reported, more so than an ERR on the command line. bye michael
Re: ole32: Add CoGetDefaultContext stub
Hallo Alistair, --- dlls/ole32/compobj.c |9 + dlls/ole32/ole32.spec |1 + 2 files changed, 10 insertions(+) diff --git a/dlls/ole32/compobj.c b/dlls/ole32/compobj.c index 0055d15..2ea1dc9 100644 --- a/dlls/ole32/compobj.c +++ b/dlls/ole32/compobj.c @@ -4279,6 +4279,15 @@ HRESULT WINAPI CoGetContextToken( ULONG_PTR *token ) return S_OK; } +/*** + * CoGetContextToken [OLE32.@] copy and paste error ^^^ + */ +HRESULT CoGetDefaultContext(APTTYPE type, REFIID riid, LPVOID *ppv) bye michael
Re: [PATCH 1/2] oleaut32/tests: Don't take the size of a pointer (Clang).
Hello Charles,
On 09/18/2012 07:51 AM, Charles Davis wrote:
From: Charles Davis cda...@mymail.mines.edu
Contrary to what a novice C programmer might expect, when you declare an
array parameter to a function, what you actually get is a pointer.
Therefore, using sizeof() on the parameter will return the size of a
pointer, and not 8*sizeof(UINT) as was intended here. Since the array was
please fix it in a way to not confuse a novice C programmer;
e.g. passing it a pointer to a MYSTRUCT. That way you can avoid the
magic constant 8 too.
explicitly declared as being 8 elements long, just use the number 8 in
the for loop instead.
---
dlls/oleaut32/tests/tmarshal.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/dlls/oleaut32/tests/tmarshal.c b/dlls/oleaut32/tests/tmarshal.c
index be8ef94..ad56123 100644
--- a/dlls/oleaut32/tests/tmarshal.c
+++ b/dlls/oleaut32/tests/tmarshal.c
@@ -596,10 +596,10 @@ static HRESULT WINAPI Widget_VarArg(
}
-static BOOL mystruct_uint_ordered(UINT uarr[8])
+static BOOL mystruct_uint_ordered(UINT uarr[])
{
int i;
-for (i = 0; i sizeof(uarr) / sizeof(uarr[0]); i++)
+for (i = 0; i 8; i++)
if (uarr[i] != i)
return 0;
thanks
bye
michael
Re: ddraw: Don't print a fixme for the flags in DirectDrawEnumerateExA; they don't change the outcome
After talking to Henri on irc, please ignore this patch. There are
patches out there to enumerate more devices than the primary one and
then the flags do play a role.
On 09/12/2012 11:08 PM, Michael Stefaniuc wrote:
---
dlls/ddraw/main.c |5 +
1 files changed, 1 insertions(+), 4 deletions(-)
diff --git a/dlls/ddraw/main.c b/dlls/ddraw/main.c
index 61e8ec9..b79b12e 100644
--- a/dlls/ddraw/main.c
+++ b/dlls/ddraw/main.c
@@ -377,12 +377,9 @@ HRESULT WINAPI
DirectDrawEnumerateExA(LPDDENUMCALLBACKEXA Callback, void *Contex
DDENUM_NONDISPLAYDEVICES))
return DDERR_INVALIDPARAMS;
-if (Flags)
-FIXME(flags 0x%08x not handled\n, Flags);
-
TRACE(Enumerating default DirectDraw HAL interface\n);
-/* We only have one driver by now */
+/* We have only one driver (primary) which is always enumerated
regardless of the flags */
__TRY
{
static CHAR driver_desc[] = DirectDraw HAL,
bye
michael
Re: Moving pages to/from wiki
On 08/30/2012 06:05 PM, Jeremy Newman wrote: On 08/29/2012 09:32 PM, Kyle Auble wrote: just checked http://cvs.winehq.org and sure enough, there is still a very outdated CVS repository there, but I can't find a template or a PHP applet for it, just the same links you found (most in WWN issues). I've CCed Jeremy Newman because I'm guessing the site admin would have to remove it. I am not opposed to removing it. We had left it up as a historical archive, or for people that were not ready to switch to GIT yet. If there is consensus then I can certainly take it down. According to Alexandre it is used by http://source.winehq.org/ for the Wine Cross Reference. bye michael
Re: [01/18] windowscodecs: Do not assume that vtable is the first element of the object.
Hello Dmitry,
On 07/24/2012 08:01 AM, Dmitry Timoshkov wrote:
The patches in this series do not depend on each other, numeration is just
for a convenience. Patches do basically the same job for different objects,
so I decided to not invent a new subject for every separate kind of object.
---
dlls/windowscodecs/bmpdecode.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/dlls/windowscodecs/bmpdecode.c b/dlls/windowscodecs/bmpdecode.c
index 6da6a3c..1a1ee0a 100644
--- a/dlls/windowscodecs/bmpdecode.c
+++ b/dlls/windowscodecs/bmpdecode.c
@@ -94,6 +94,8 @@ static inline BmpDecoder
*impl_from_IWICBitmapFrameDecode(IWICBitmapFrameDecode
static HRESULT WINAPI BmpFrameDecode_QueryInterface(IWICBitmapFrameDecode
*iface, REFIID iid,
void **ppv)
{
+BmpDecoder *This = impl_from_IWICBitmapFrameDecode(iface);
+
TRACE((%p,%s,%p)\n, iface, debugstr_guid(iid), ppv);
if (!ppv) return E_INVALIDARG;
@@ -102,7 +104,7 @@ static HRESULT WINAPI
BmpFrameDecode_QueryInterface(IWICBitmapFrameDecode *iface
IsEqualIID(IID_IWICBitmapSource, iid) ||
IsEqualIID(IID_IWICBitmapFrameDecode, iid))
{
-*ppv = iface;
+*ppv = This-IWICBitmapFrameDecode_iface;
}
else
{
this part of the change is gratuitous. As long as there is only one
interface implementation in the object there is no need for impl_from_Foo().
bye
michael
Re: quartz/tests: Add COM aggregation test for NullRenderer. (2nd try)
On 07/02/2012 01:32 AM, Marvin wrote: 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=19686 Your paranoid android. === WNT4WSSP6 (32 bit misc) === misc.c:60: Test failed: CoCreateInstance failed with 80040154 misc.c:61: Test failed: pUnkOuter is NULL No quartz.dll on that box. bye michael
Re: [PATCH 2/2] qedit/tests: Add COM aggregation test for MediaDet.
On 07/01/2012 12:12 AM, Marvin wrote: 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=19677 Your paranoid android. === WNT4WSSP6 (32 bit mediadet) === mediadet.c:149: Test failed: CoCreateInstance failed: 80040154 mediadet: unhandled exception c005 at 00401587 === W2KPROSP4 (32 bit mediadet) === mediadet.c:149: Test failed: CoCreateInstance failed: 80040154 mediadet: unhandled exception c005 at 00401587 Both hosts miss qedit.dll. bye michael
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: 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 mstef...@redhat.com 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.
On 06/28/2012 01:50 PM, Dmitry Timoshkov wrote: Michael Stefaniuc mstef...@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-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: [PATCH 3/4] qedit/tests: Add COM aggregation test for MediaDet.
Argh!
Please skip this patch, it is incomplete and just skips the rest of the
tests on Wine. I'll resubmit it when I implement COM aggregation for
MediaDet. Patch 4/4 can be still applied as it doesn't depends on this.
bye
michael
On 06/26/2012 11:53 PM, Michael Stefaniuc wrote:
---
dlls/qedit/tests/mediadet.c |7 +++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/dlls/qedit/tests/mediadet.c b/dlls/qedit/tests/mediadet.c
index 1d9a2f1..1d75680 100644
--- a/dlls/qedit/tests/mediadet.c
+++ b/dlls/qedit/tests/mediadet.c
@@ -132,6 +132,7 @@ static BOOL init_tests(void)
static void test_mediadet(void)
{
HRESULT hr;
+struct unk_impl unk_obj = {{unk_vtbl}, 19, NULL};
IMediaDet *pM = NULL;
BSTR filename = NULL;
LONG nstrms = 0;
@@ -141,6 +142,12 @@ static void test_mediadet(void)
int flags;
int i;
+/* COM aggregation */
+hr = CoCreateInstance(CLSID_MediaDet, unk_obj.IUnknown_iface,
CLSCTX_INPROC_SERVER,
+IID_IUnknown, (void**)unk_obj.inner_unk);
+todo_wine ok(hr == S_OK, CoCreateInstance failed: %08x\n, hr);
+if (hr != S_OK) return;
+
/* test.avi has one video stream. */
hr = CoCreateInstance(CLSID_MediaDet, NULL, CLSCTX_INPROC_SERVER,
IID_IMediaDet, (LPVOID*)pM);
Re: [PATCH 4/4] qedit: Support COM aggregation for SampleGrabber.
On 06/27/2012 12:02 AM, Marvin wrote: 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? No qedit.dll on those two boxes. Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=19578 Your paranoid android. === WNT4WSSP6 (32 bit mediadet) === mediadet.c:148: Test failed: CoCreateInstance failed: 80040154 mediadet.c:356: Test failed: CoCreateInstance failed: 80040154 mediadet: unhandled exception c005 at 00401639 === W2KPROSP4 (32 bit mediadet) === mediadet.c:148: Test failed: CoCreateInstance failed: 80040154 mediadet.c:356: Test failed: CoCreateInstance failed: 80040154 mediadet: unhandled exception c005 at 00401639 bye michael
Re: Wine automated testing update
Francois, On 06/21/2012 04:21 PM, Francois Gouget wrote: --- Network trouble I'm having trouble getting the network to work fine in the QEmu VMs. Eventually I discovered that if I bridged to eth0 then it's just the communication between the host and the VM that does not work. Communication with other machines on the LAN or with the Internet works. However that's going to be a big problem to send the jobs to the VMs (see previous section, plus the dhcp and dns is also running on the host in my case). Now, rather than bridge to eth0 what I'd want on my machine is to bridge to tap0, a local network where I have all my VMware VMs. can you please send me the network details of the host? Normally you don't bridge to eth0 but create a bridge interface and eth0 is a member of that bridge aka eth0 won't have an IP associated with it as the host IP would be on virtbr0. What I typically see if I ping the host from the VM is that the echo requests go out and the echo replies are sent. But ping on the VM sees nothing. It could be a firewalling issue though my VMware VMs on the same network work fine. bye michael
Re: 'Expensive' expressions in loop limits (was: Implementation of D3DXSHScale)
On 06/19/2012 10:47 AM, Dan Kegel wrote: Nozomi wrote: +for (i = 0; i order * order; i++) I might have written int n = order * order; for (i=0; i n; i++) to avoid repeating the multiplication every time around the loop, even though multiplication is cheap nowadays, and -O1 will optimize it out anyway. Staying in the habit of avoiding 'expensive' operations in loop limits might still be a good idea, since the optimizer can't always save you. Or is that considered ugly these days? Of course as always a definitive It depends. The safe approach is to *first* optimize for the human reader; the compilers this days are better at optimizing than the average programmer. bye michael
Re: MSVCP60.dll
Hello John! On 06/15/2012 06:22 AM, John Emmas wrote: Firstly, I'm not a Linux user. I'm a Windows programmer but I have a passing knowledge of Linux (and several friends who are Linux programmers). I write a Windows application which gets launched as a child process by a popular Linux DAW. My program was first written many years ago when the (then) current version of Wine was about v0.9.58. Over a hundred people are using my program with old versions of Wine and I've had no complaints so far. Recently however, two customers tried to use it with Wine v1.5.35. In both cases the program crashed - apparently because some particular function wasn't found in MSVCP60.dll (at the moment, we don't know which function). Both customers went to a web site called WineTricks from That is fairly trivial to figure out. Start your app with Wine on the command line and it will crash with an exception: Call from address to unimplemented function MSVCP60.function Once you have those please open a bug on http://bugs.winehq.org/ for them. where they were able to install an alternative version of MSVCP60.dll. My app now gets past the place where it previously crashed although it now crashes further on... :-( It looks as if the MSVC6 runtime module(s) that now come with Wine aren't the same as the ones from v0.9.58. Either they're different MSVCP60.dll wasn't available in wine-0.9.58. It was introduced in wine-1.3.19. They were probably using the dll that came with your app or installed it from somewhere else. versions of the Microsoft DLLs or maybe they're now being implemented by somebody else, perhaps as part of Wine development? Either way, they don't work with my particular app any more. Please open a bug for those issues too. I still have Visual C++6 installed on an old computer, together with both the source code and the (InstallShield) packager for my original app. So is there anything I can do to make this work? For example, do Open the bug reports :) If there is a free downloadable version/demo that exhibits the same issues please add it to the URL field. bye michael
Re: wined3d: Avoid sizeof on structs with variable length arrays.
On 06/13/2012 12:56 PM, Henri Verbeet wrote: On 13 June 2012 10:41, Michael Stefaniuc mstef...@redhat.de wrote: -object = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*object)); +object = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, FIELD_OFFSET(struct wined3d, adapters[1])); That's not really a VLA, just a somewhat stupid array of size 1. Uh... Any reason that field cannot be just struct wined3d_adapter adapter; ? bye michael
Re: A copyright question to wine developpers
Hello! On 06/11/2012 10:54 AM, Christophe-Marie Duquesne wrote: To my understanding, wine is a reimplementation of the MS system. As far as I understand, you take MS public headers and reimplement their functions. If that is how it works, then how do you deal with copyright? The MS headers certainly come with a copyright clause: how is it possible to redistribute these headers with wine? The Windows headers are copyrighted and Wine cannot redistribute those. Wine provides its own headers. bye michael
Re: http://winetricks.googlecode.com/svn/trunk/src/install-gecko.sh also installs mono
On 06/05/2012 08:53 AM, Max TenEyck Woodbury wrote: The issue is access from linux native code to the .net framework. That should require a specific decision on the part of the system administrator to make it available. It is that package that I believe is called 'mono'. I have taken steps to assure that it never appears on the systems I run. Providing a .net framework to MSWindows platform applications that I want to run on my systems is another mater. Wine should provide that framework on demand but that is NOT 'mono'. The Wine .net support does NOT make the .net framework available to native linux application. (If it That's *exactly* what wine-mono *IS*. Unlike before this is the Windows version of mono and thus unusable from Linux. IS provided a part of the wine migration library, I will grumble but not scream about it.) bye michael
Re: quartz: COM cleanup for Parser_OutputPin
Hello Aric,
On 05/18/2012 04:27 PM, Aric Stewart wrote:
diff --git a/dlls/quartz/avisplit.c b/dlls/quartz/avisplit.c
index 5027e90..bac1b24 100644
--- a/dlls/quartz/avisplit.c
+++ b/dlls/quartz/avisplit.c
@@ -288,7 +288,7 @@ static HRESULT AVISplitter_next_request(AVISplitterImpl
*This, DWORD streamnumbe
static HRESULT AVISplitter_Receive(AVISplitterImpl *This, IMediaSample
*sample, DWORD streamnumber)
{
-Parser_OutputPin *pin = (Parser_OutputPin
*)This-Parser.ppPins[1+streamnumber];
+Parser_OutputPin *pin =
impl_Parser_OutputPin_from_IPin(This-Parser.ppPins[1+streamnumber]);
this isn't right. impl_from_... should be used only from methods of the
respective implementation. You can either:
- Make This-Parser.ppPins an array of Parser_OutputPin instead of IPin
- Or if you need to support application provided IPin then you'll need
an unsafe_impl_from_...
HRESULT hr;
LONGLONG start, stop, rtstart, rtstop;
StreamData *stream = This-streams[streamnumber];
bye
michael
Re: mstask: Actually run a test and fix the expected behaviour.
On 05/16/2012 11:32 AM, Marvin wrote: 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=18419 Your paranoid android. === WNT4WSSP6 (32 bit task_trigger) === task_trigger.c:165: Test failed: Failed to setup test_task That box has no mstask dll. bye michael
Re: msxml3: avoid a dangling else (LLVM/Clang)
Hello Austin,
On 05/14/2012 08:01 AM, Austin English wrote:
diff --git a/dlls/msxml3/domdoc.c b/dlls/msxml3/domdoc.c
index 6805c75..14fd01b 100644
--- a/dlls/msxml3/domdoc.c
+++ b/dlls/msxml3/domdoc.c
@@ -2277,8 +2277,10 @@ static HRESULT WINAPI domdoc_loadXML(
/* skip leading spaces if needed */
if (This-properties-version == MSXML_DEFAULT ||
This-properties-version == MSXML26)
+{
while (*ptr)
if (isspaceW(*ptr)) ptr++; else break;
I do not mind terse code but that kinda overdoes it. I would put the
else on a separate line.
+}
xmldoc = doparse(This, (char*)ptr, strlenW(ptr)*sizeof(WCHAR),
XML_CHAR_ENCODING_UTF16LE);
if ( !xmldoc )
bye
michael
Re: msxml3: avoid a dangling else (LLVM/Clang)
On 05/14/2012 11:47 AM, Eric Pouech wrote: while (*ptr) if (isspaceW(*ptr)) ptr++; else break; I do not mind terse code but that kinda overdoes it. I would put the else on a separate line. trafic on wine-devel is rather low those days what about opening a code-style flame-war ? Heh, no, that wasn't my intend. With the if and else on separate lines it would have been obvious that the curly brackets would be better placed inside the while... bye michael
Re: Patch 85420
On 04/17/2012 09:46 PM, Marcus Meissner wrote: On Tue, Apr 17, 2012 at 09:10:45PM +0200, Christian Costa wrote: Patch http://source.winehq.org/patches/data/85420 is marked as pending. I've taken a look but I still don't understand what's wrong. Is there anything I've missed ? I stopped reading at this line: +*frame = (LPDIRECT3DRMFRAME)(impl_from_IDirect3DRMFrame3(This-parent)-IDirect3DRMFrame2_iface); You are doing something very wrong if you need to cast this way. That's just the stupid obfuscation introduced by using LPDIRECT3DRMFRAME instead of (IDirect3DRMFrame *) and then the cast might be plausible (e.g. a function taking an IFace* type but which is implemented by the IFace2 interface). There are three other issues with that line though that kill it: - For type safety reasons impl_from_IFace() needs to be used *only* in variable declarations. - Calling impl_from_IFace() on interface pointers outside methods that implement that interface are verboten. - Both above points can be completely avoided by making the parent field be IDirect3DRMFrameImpl *parent. It probably also needs explicit sign-off drom the d3d folks as it handles funny refcounting. Not really, this is plain COM stuff. bye michael
Re: [PATCH 1/3] browseui: Add IOleWindow to IProgressDialog
Hello Detlef,
On 04/15/2012 10:08 PM, Detlef Riekenberg wrote:
Used by apps to adjust the dialog position
or remove the cancel button before vista
--
By by ... Detlef
---
dlls/browseui/progressdlg.c | 69
++-
1 files changed, 68 insertions(+), 1 deletions(-)
diff --git a/dlls/browseui/progressdlg.c b/dlls/browseui/progressdlg.c
index 9b970e2..2807622 100644
--- a/dlls/browseui/progressdlg.c
+++ b/dlls/browseui/progressdlg.c
@@ -270,9 +276,20 @@ static HRESULT WINAPI
ProgressDialog_QueryInterface(IProgressDialog *iface, REFI
ProgressDialog *This = impl_from_IProgressDialog(iface);
*ppvOut = NULL;
-if (IsEqualIID(iid, IID_IUnknown) || IsEqualIID(iid,
IID_IProgressDialog))
+if (IsEqualIID(iid, IID_IUnknown))
{
*ppvOut = This;
+TRACE((%p, IID_IUnknown, %p) - %p\n, This, ppvOut, This);
+}
+else if (IsEqualIID(iid, IID_IProgressDialog))
+{
+*ppvOut = This;
this is not correct as ppvOut takes an interface and not an object.
While at it please fix the above one too.
thanks
bye
michael
Re: ddraw/tests: Compare the correct surface pointers.
On 04/15/2012 10:04 PM, Marvin wrote: While running your changed tests on Windows, I think I found new failures. Yupp, test is incorrect. Stefan is looking at it. bye michael 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=17873 Your paranoid android. === W2KPROSP4 (32 bit dsurface) === dsurface.c:272: Test failed: lpSurface from GetSurfaceDesc() differs from the one returned by Lock(00A70028) === WXPPROSP3 (32 bit dsurface) === dsurface.c:272: Test failed: lpSurface from GetSurfaceDesc() differs from the one returned by Lock(00BA0028) === W2K3R2SESP2 (32 bit dsurface) === dsurface.c:272: Test failed: lpSurface from GetSurfaceDesc() differs from the one returned by Lock(00E40028) === WVISTAADM (32 bit dsurface) === dsurface.c:272: Test failed: lpSurface from GetSurfaceDesc() differs from the one returned by Lock(02210028) === W2K8SE (32 bit dsurface) === dsurface.c:272: Test failed: lpSurface from GetSurfaceDesc() differs from the one returned by Lock(02800028) === W7PRO (32 bit dsurface) === dsurface.c:272: Test failed: lpSurface from GetSurfaceDesc() differs from the one returned by Lock(01BF0028) === W7PROX64 (32 bit dsurface) === dsurface.c:272: Test failed: lpSurface from GetSurfaceDesc() differs from the one returned by Lock(02740028) === TEST64_W7SP1 (32 bit dsurface) === dsurface.c:272: Test failed: lpSurface from GetSurfaceDesc() differs from the one returned by Lock(02690028) === W7PROX64 (64 bit dsurface) === dsurface.c:272: Test failed: lpSurface from GetSurfaceDesc() differs from the one returned by Lock(02390040) === TEST64_W7SP1 (64 bit dsurface) === dsurface.c:272: Test failed: lpSurface from GetSurfaceDesc() differs from the one returned by Lock(02360040)
Re: [PATCH 1/3] amstream: Add CLSID_AMAudioData implementation. (try 2)
Hello Christian,
On 04/03/2012 07:53 AM, Christian Costa wrote:
+/*** IUnknown methods ***/
+static HRESULT WINAPI IAudioDataImpl_QueryInterface(IAudioData *iface,
REFIID riid, void **ppv)
+{
+AMAudioDataImpl *This = impl_from_IAudioData(iface);
+
+TRACE((%p/%p)-(%s,%p)\n, iface, This, debugstr_guid(riid), ppv);
+
+if (IsEqualGUID(riid, IID_IUnknown) ||
+IsEqualGUID(riid, IID_IMemoryData) ||
+IsEqualGUID(riid, IID_IAudioData))
+{
+IUnknown_AddRef(iface);
+*ppv = This;
even though MS loves to call ppv ppvObject it isn't an object but a
pointer to an interface. As this object has just one interface you can
get rid of This and just assign iface to *ppv.
+return S_OK;
+}
+
+ERR((%p)-(%s,%p),not found\n, This, debugstr_guid(riid), ppv);
+return E_NOINTERFACE;
+}
+
bye
michael
Re: [PATCH 1/4] dmime: COM cleanup for IDirectMusicPerformance8.
Hello Christian, On 04/02/2012 09:05 AM, Christian Costa wrote: While you're at it, could you put a 4 spaces indentation to whole method. I thought about that but that would make the patch huge and it is already big. The re-indentation can be done when the functions get implemented. bye michael -- Michael Stefaniuc Tel.: +49-711-96437-199 Consulting Communications Engineer Fax.: +49-711-96437-111 Reg. Adresse: Red Hat GmbH, Werner-von-Siemens-Ring 14, 85630 Grasbrunn Handelsregister: Amtsgericht Muenchen HRB 153243 Geschäftsführer: Mark Hegarty, Charlie Peters, Michael Cunningham, Charles Cachera
Re: [PATCH 3/3] d3drm: Implement CreateMesh method and stubbed IDirect3DRMMesh interface.
Hello Christian,
On 03/29/2012 11:57 PM, Christian Costa wrote:
+HRESULT Direct3DRMMesh_create(REFIID riid, IUnknown** ppObj)
+{
+IDirect3DRMMeshImpl* object;
+
+TRACE((%p)\n, ppObj);
+
+object = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY,
sizeof(IDirect3DRMMeshImpl));
+if (!object)
+{
+ERR(Out of memory\n);
+return E_OUTOFMEMORY;
+}
+
+object-IDirect3DRMMesh_iface.lpVtbl = Direct3DRMMesh_Vtbl;
+object-ref = 1;
+
+if (IsEqualGUID(riid, IID_IDirect3DRMMesh))
+*ppObj = (IUnknown*)object-IDirect3DRMMesh_iface;
+else
+*ppObj = (IUnknown*)object-IDirect3DRMMesh_iface;
both if and else code path are the same.
+
+return S_OK;
+}
+
bye
michael
Re: 'Pending' patches state
Alexandre, On 03/28/2012 10:17 AM, Alexandre Julliard wrote: Dmitry Timoshkov dmi...@baikal.ru writes: It's very confusing, and absolutely not clear what is required from the patch submitter, especially since *there is no any feedback on the patch*. 'Rejected' at least requies some sort of feedback, while 'Pending' doesn't. To me 'Pending' looks like a silent case of 'Reject', but without any obligation to explain why. I find myself on somewhat shaky ground when I see a bunch of my patches in the pending state, especially if they already contain the tests and main developer in the area I'm changing is Alexandre :). Is it possible to get at least some feedback for pending patches? Pretty please? The pending state is feedback. It means that the patch is not clearly yes, but the worst possible feedback. New people assume you or the area maintainer need to still make up their mind on the patch but that's not the case, it is a done deal. correct, but that it's complicated to articulate exactly why. Like it says, you should try to make it more convincing, either by simplifying the patch, or writing a test case. Iff one knows you and knows to read in between the lines then the above can be reworded as: Current implementation rejected; idea might have some merit. That's what Pending means most of the times. Now for the other meanings of Pending: - Waiting for feedback from the main developer in that area. Don't remember to have ever seen that. Those patches stay normally in New as you don't look at them before the area developer ACKs. - preferably in the form of a test case That should be Needs tests. - With the two (minor) other meanings of Pending handled by other patch states we can rename Pending to Convince me or Needs work. That would make it more obvious what is meant. For instance your patch 84692 says that tests confirm that, but you don't say which tests, and there are no new tests or fixed todos in the patch, so it looks suspicious. Yes, I could dig out the tests myself and investigate it in detail, but when it gets to that point I usually just move on to the next patch, hence pending. IMHO that should have been Needs tests; that would have forced Dmitry to point you to the tests. bye michael
Re: d3d10core: Standardize COM aggregation for d3d10_device.
Now that Alexandre is back and Jacek didn't want to beat me to it it is
time to finish my reply...
On 03/22/2012 01:06 AM, Henri Verbeet wrote:
On 22 March 2012 00:50, Michael Stefaniuc mstef...@redhat.de wrote:
-static HRESULT STDMETHODCALLTYPE d3d10_device_inner_QueryInterface(IUnknown
*iface, REFIID riid, void **object)
+static HRESULT STDMETHODCALLTYPE d3d10_device_inner_QueryInterface(IUnknown
*iface, REFIID riid,
+void **ppv)
{
-struct d3d10_device *This = d3d10_device_from_inner_unknown(iface);
+struct d3d10_device *This = impl_from_IUnknown(iface);
-TRACE(iface %p, riid %s, object %p\n, iface, debugstr_guid(riid),
object);
+TRACE(iface %p, riid %s, object %p\n, iface, debugstr_guid(riid),
ppv);
-if (IsEqualGUID(riid, IID_IUnknown)
-|| IsEqualGUID(riid, IID_ID3D10Device))
+if (IsEqualGUID(riid, IID_IUnknown) || IsEqualGUID(riid,
IID_ID3D10Device))
+*ppv = This;
+else if (IsEqualGUID(riid, IID_IWineDXGIDeviceParent))
+*ppv = This-IWineDXGIDeviceParent_iface;
+else
{
-ID3D10Device_AddRef(This-ID3D10Device_iface);
-*object = This;
-return S_OK;
+WARN(%s not implemented, returning E_NOINTERFACE\n,
debugstr_guid(riid));
+*ppv = NULL;
+return E_NOINTERFACE;
}
-if (IsEqualGUID(riid, IID_IWineDXGIDeviceParent))
-{
-IWineDXGIDeviceParent_AddRef(This-IWineDXGIDeviceParent_iface);
-*object = This-IWineDXGIDeviceParent_iface;
-return S_OK;
-}
-
-WARN(%s not implemented, returning E_NOINTERFACE\n,
debugstr_guid(riid));
-
-*object = NULL;
-return E_NOINTERFACE;
+IUnknown_AddRef((IUnknown*)*ppv);
+return S_OK;
}
I'm not sure this really makes it much better, but I guess it's mostly
up to how Alexandre wants these.
Honestly, when Jacek documented it in the beginning I didn't like it
either. But having to deal now with the bad, the ugly and the censored
COM implementations in Wine I have overcome my hate for casts and
actually like this way more and more.
Of course if people know COM then it doesn't matter much. But looking at
COM in Wine, feeling the pain of some native COM implementations and how
the applications abuse them it is save to assume that most
developers that write COM stuff don't bother to learn COM.
COM is programmed by cutpaste and beating the code into submission. And
for that use case the way that Jacek documented is way better:
- Generic solution which works in all cases:
* Single interface implementation in the object (not the most elegant
solution though).
* Multiple interface implementations with one refcount.
* Multiple interface implementations with multiple refcounts.
- AddRef in one and only one place which is very explicit on what needs
to be addref'ed (the returned interface). And most importantly the
AddRef is part of the boiler plate which doesn't needs to change during
cutpaste programming.
- And last but not least the 3rd parameter of QueryInterface doesn't
have object in its name. It is *not* an object even though MS loves to
call it ppvObject. ppv is just peepeevee; any resemblance of
Hungarian notation is pure coincidence ;). Though I don't mind changing
it as long as it doesn't contains obj/object.
bye
michael
Re: mscoree: Print the debug string and not the pointer to it.
On 03/23/2012 07:50 PM, Lauri Kenttä wrote:
On 2012-03-22 00:36, Michael Stefaniuc wrote:
HRESULT WINAPI GetRequestedRuntimeVersion(LPWSTR pExe, LPWSTR
pVersion, DWORD cchBuffer, DWORD *dwlength)
{
- TRACE((%s, %p, %d, %p)\n, debugstr_w(pExe), debugstr_w(pExe),
cchBuffer, dwlength);
+ TRACE((%s, %s, %d, %p)\n, debugstr_w(pExe), debugstr_w(pExe),
cchBuffer, dwlength);
Why is pExe printed two times? Maybe the second one should be pVersion?
Good catch. Please feel free to sent a patch that fixes that and
supersedes my patch.
thanks
bye
michael
Re: Updating GSoC proposal
Hello Alex, On 03/20/2012 05:17 PM, HolyCause wrote: On 3/20/12 07:22, Maarten Lankhorst wrote: Agreed, would like to add cmd parser to that list too. ~Maarten Why is this? Does cmd sound like a bad project? FWIW I was planning on improving cmd as my GSoC project, and I've talked to Dan Kegel about it - there's been no protests (only support) thus far on that front, so it at least seems doable. Writing a proper cmd parser is doable. Getting that into Wine in small and incremental patches will be hard as a big drop in patch is not an option. In terms of being valid... if cmd were improved it would allow firefox and chromium to be built under wine (if reg was also improved a bit, but that might also fall under my project...) Improving cmd should be fine as a project. Hoping to get the parser fully replaced during that isn't. bye michael
Re: Networking+wine
On 03/19/2012 01:40 PM, prateek papriwal wrote: http://wiki.winehq.org/FAQ#head-f566a12c806a1eacaeefb7cb6419a513a773c571 as mentioned wine does not provide sandboxing at all but there can be some wine users would like to prevent Windows apps from accessing unix system so there is need for a simple sandboxing interface. Like the FAQ says use SELinux or AppArmor for that. There is no way that Wine can prevent that as the Windows apps can just call into a Linux lib or even perform a direct system call. Or just remove the Z: drive in winecfg as a poor men sandboxing. On Mon, Mar 19, 2012 at 1:57 PM, Austin English austinengl...@gmail.com mailto:austinengl...@gmail.com wrote: On Mon, Mar 19, 2012 at 01:19, prateek papriwal papriwalprat...@gmail.com mailto:papriwalprat...@gmail.com wrote: Networking + wine (like as running uTorrent in ubuntu) can be very dangerous. I feel there should be some sandboxing(as in Windows) to prevent frequent attacks. I would be very interested in implementing sandboxing for running Wine applications. What sandboxing are you referring to in Windows? Regarding sandboxing Wine itself, see the FAQ entry: http://wiki.winehq.org/FAQ#head-f566a12c806a1eacaeefb7cb6419a513a773c571 bye michael
Re: [PATCH] po: Remove bad characters (C2 A0) from french translations.
On 03/13/2012 12:42 AM, Christian Costa wrote: Le 12/03/2012 20:32, Christian Costa a écrit : Le 12/03/2012 21:02, Nikolay Sivov a écrit : On 3/12/2012 21:52, Christian Costa wrote: They are displayed as a single space in a text editor but visible with an hex editor. I guess this is a non-breakable space. I don't know. Anyway it produces garbage for what the string is used for so the patch still applies. I will resend the patch with a description that reflects that. BTW, is there an editor that shows them ? gvim doesn't. I see them as normal space in vim but searching for a normal space doesn't highlight them. You can use the highlight and match command to show them in a different color. Non-breakable space are only present in french translations. Between a name and a ':'. This is not nicer to me. I wondering if that would not be better to remove them all... It doesn't matter if you consider them nicer or not. If they are required by the rules of the language then they will stay. bye michael
Re: use assert etc. in Wine dlls? What is better?
Hello Joerg,
On 03/13/2012 03:05 PM, joerg-cyril.hoe...@t-systems.com wrote:
Maarten's wine-pulse contains several instances of assert()
I've been thinking about adding a bit of protection against
inconcsistencies within mmdevapi code too.
There are two variants that could be applicable depending on the situation:
/* this should not happen, but if we detect it early, we can work-around the
situation */
if (a limit) { ERR(%d %d\n, a, limit); a = limit; }
or
assert(a = limit, whatever %d, a);
I'm not too fond of assert. An assert in DSound was one of my first bug
fixes in Wine, years ago. It was not obvious why the app should not
continue running -- perhaps without sound.
OTOH, if an assert is violated, as a programmer, I'd like to hear about it.
I found little on that topic in Wine. In 2002, Alexandre Julliard wrote:
http://www.winehq.org/pipermail/wine-devel/2002-October/009766.html
A better approach would be to leave assert() alone, and raise an
exception on the SIGABRT signal.
What is current opinion on this topic?
there are quite a few assert calls used in the dlls:
git grep -w assert dlls | grep -v include | wc -l
1509
But of course the correct answer when to add an assert is as always It
depends ;)
- If some internal state got corrupt and the app has no way to recover
but crash later on it is better to assert as early as possible as that
simplifies the debugging.
- If the app has a chance to deal with the error then returning an error
is the better approach. Add an ERR() or WARN() to that so we at least
can see the issue.
bye
michael
Re: Got Application cross-compiled with msvc11 for ARM running
On 03/08/2012 05:40 PM, André Hentschel wrote: Hi, I cross-compiled a simple Application with the preview of MSVC 11 to ARM. After some minor changes to Wine i was able to run it on my Pandaboard using the native msvcr110.dll. One thing i noticed in the cross-compile environment is that there are much less libraries available for ARM, that's mostly because the win32 API isn't meant to be ported entirely, instead ARM Apps should be based on WinRT. Still i was able to dynamic load functions from dlls available in Wine beside static loaded functions that also worked. In short: The good news: The ABI is compatible (to armel gnueabi) The bad news: no fully win32 API on ARM Bad news? Quite the contrary. Good use case for Wine on Windows. Maybe my small changes to Wine will make it into 1.5.1 :) At least i hope so. 1.5.1 or 1.4.1? bye michael
Re: Got Application cross-compiled with msvc11 for ARM running
On 03/08/2012 05:54 PM, André Hentschel wrote: Am 08.03.2012 17:52, schrieb Michael Stefaniuc: On 03/08/2012 05:40 PM, André Hentschel wrote: Hi, I cross-compiled a simple Application with the preview of MSVC 11 to ARM. After some minor changes to Wine i was able to run it on my Pandaboard using the native msvcr110.dll. One thing i noticed in the cross-compile environment is that there are much less libraries available for ARM, that's mostly because the win32 API isn't meant to be ported entirely, instead ARM Apps should be based on WinRT. Still i was able to dynamic load functions from dlls available in Wine beside static loaded functions that also worked. In short: The good news: The ABI is compatible (to armel gnueabi) The bad news: no fully win32 API on ARM Bad news? Quite the contrary. Good use case for Wine on Windows. If we get that running, true. I don't mean the full Wine, just the missing libs and headers for people to easily port their Windows apps to ARM. Maybe my small changes to Wine will make it into 1.5.1 :) At least i hope so. 1.5.1 or 1.4.1? I don't decide that :) Maybe both ;) bye michael
Re: winemenubuilder: Nothing useful is done on Mac OS X so just exit.
Hello Francois, On 03/01/2012 03:07 AM, Francois Gouget wrote: --- A real Mac OS X implementation would be nice but barring that it seems better to do nothing than to create .desktop files and directories that make no sense on Mac OS X. programs/winemenubuilder/winemenubuilder.c |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/programs/winemenubuilder/winemenubuilder.c b/programs/winemenubuilder/winemenubuilder.c index 1588e50..458e207 100644 --- a/programs/winemenubuilder/winemenubuilder.c +++ b/programs/winemenubuilder/winemenubuilder.c @@ -3619,6 +3619,10 @@ int PASCAL wWinMain (HINSTANCE hInstance, HINSTANCE prev, LPWSTR cmdline, int sh HRESULT hr; int ret = 0; +#ifdef __APPLE__ +WINE_FIXME(creating menus is not supported on Mac OS X\n); +return 0; +#endif if (!init_xdg()) return 1; no clue if the compiler there complains but you are producing unreachable code in the #ifdef __APPLE__ case. bye michael
Re: [PATCH 1/2] winmm: Avoid using SuspendThread, it can hang Wine.
joerg-cyril.hoe...@t-systems.com wrote:
Alexandre Julliard wrote:
Please stop abusing goto, write a proper loop instead.
No problem. The idea was to minimize the patch and show what really changed.
Adding a while loop will indent all 120 lines of code in between { }.
See that as an opportunity to clean up the whitespace issues in those
120 lines ;). Or of course to split the code out into helper functions.
bye
michael
Re: [PATCH 1/2] winmm: Avoid using SuspendThread, it can hang Wine.
joerg-cyril.hoe...@t-systems.com wrote: Michael Stefaniuc wrote: See that as an opportunity to clean up the whitespace issues in those 120 lines ;) Hmm, what was the latest word on reformatting, if any? Until now I've been careful to preserve the surrounding formatting, even Though winmmmci look very different from mmdevapi. Do you suggest to remove all TAB from lines that I touch, even if only re-indenting? That. And getting rid of trailing/dangling whitespace, placing the whitespace consistently around operators, etc. bye michael
Re: winecfg: Center main window on the screen.
Vitaliy Margolen wrote: Please don't! I want my window manager decide based on how I configured it where to position new windows. And not every program open in the middle of the screen just because it things it's the most important thing on my screen. Uh? At the moment it always opens in the top right corner. There doesn't seems to be a way in Win32 land to not specify a position where to open the window. bye michael
Re: comctl32: Remove an obsolete resource attribute.
Marvin wrote: 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=16849 Your paranoid android. === WNT4WSSP6 (32 bit) === No test summary line found WTB is confused by the change to a non-c file and didn't run any tests. bye michael
Re: po: Remove untranslated English strings from the Romanian translation
Hello Frédéric, the patch is wrong. Frédéric Delanoy wrote: --- po/ro.po |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/po/ro.po b/po/ro.po index a0912c2..c9375c4 100644 --- a/po/ro.po +++ b/po/ro.po @@ -11803,11 +11803,11 @@ msgstr Bază #: regedit.rc:282 msgid Hexadecimal -msgstr Hexadecimal +msgstr #: regedit.rc:283 msgid Decimal -msgstr Decimal +msgstr Those aren't untranslated but the Romanian words match the English ones. #: regedit.rc:290 msgid Edit Binary @@ -13243,7 +13243,7 @@ msgstr Afișează opțiunile avansate #: winecfg.rc:240 msgid Device: -msgstr Device: +msgstr #: winecfg.rc:242 msgid Browse... bye michael
Re: po: Remove untranslated English strings from the Romanian translation
Francois Gouget wrote: On Tue, 7 Feb 2012, Michael Stefaniuc wrote: [...] #: regedit.rc:282 msgid Hexadecimal -msgstr Hexadecimal +msgstr #: regedit.rc:283 msgid Decimal -msgstr Decimal +msgstr Those aren't untranslated but the Romanian words match the English ones. Are you sure? If I look at Wikipedia it looks like they're spelled Hexazecimal and Zecimal respectively: Of course I'm sure until proven wrong! ;) http://ro.wikipedia.org/wiki/Sistem_zecimal I could swear to have never seen that word before. But I left Romania 20 years ago so things might have changed. That said I'm all ears for false positives to improve the Wine-PO reports. http://fgouget.free.fr/wine-po/ bye michael
Re: snd_card_get_name uses strdup (Valgrind).
Hello Joerg,
On Wed, Feb 01, 2012 at 01:37:44PM +0100, joerg-cyril.hoe...@t-systems.com
wrote:
there's a small memory leak in winealsa, tentatively plugged by
patches #83353, #83334 and #82900.
It would be nice if somebody who feels like struggling with const warnings in
gcc
would get a fix accepted in git.
The dilemma: a literal foo is a const char*, free() does not want a const
and casts are the root of all evil in C.
something like the attached patch should be more palatable.
bye
michael
From 6befb1255dd8217c038a07f5da476a3a0e2ad870 Mon Sep 17 00:00:00 2001
From: Michael Stefaniuc mstef...@redhat.de
Date: Wed, 1 Feb 2012 14:45:25 +0100
Subject: winealsa.drv: Clean up the sound card name retrieval code.
Fixes a memleak found by Valgrind.
---
dlls/winealsa.drv/mmdevdrv.c | 28
1 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/dlls/winealsa.drv/mmdevdrv.c b/dlls/winealsa.drv/mmdevdrv.c
index 0bedcc5..46adb56 100644
--- a/dlls/winealsa.drv/mmdevdrv.c
+++ b/dlls/winealsa.drv/mmdevdrv.c
@@ -363,8 +363,9 @@ static HRESULT alsa_enum_devices(EDataFlow flow, WCHAR
**ids, char **keys,
for(err = snd_card_next(card); card != -1 err = 0;
err = snd_card_next(card)){
char cardpath[64];
-const char *cardname;
+char *cardname;
WCHAR *cardnameW;
+WCHAR szcardname[] = {'U','n','k','n','o','w','n','
','s','o','u','n','d','c','a','r','d'};
snd_ctl_t *ctl;
DWORD len;
@@ -376,24 +377,27 @@ static HRESULT alsa_enum_devices(EDataFlow flow, WCHAR
**ids, char **keys,
continue;
}
-if((err = snd_card_get_name(card, (char **)cardname)) 0){
+if(!(err = snd_card_get_name(card, cardname))){
+len = strlen(cardname);
+cardnameW = HeapAlloc(GetProcessHeap(), 0, len * sizeof(WCHAR));
+if(!cardnameW){
+free(cardname);
+snd_ctl_close(ctl);
+return E_OUTOFMEMORY;
+}
+MultiByteToWideChar(CP_UNIXCP, 0, cardname, len, cardnameW, len);
+free(cardname);
+}else{
WARN(Unable to get card name for ALSA device %s: %d (%s)\n,
cardpath, err, snd_strerror(err));
/* FIXME: Should be localized */
-cardname = Unknown soundcard;
-}
-
-len = MultiByteToWideChar(CP_UNIXCP, 0, cardname, -1, NULL, 0);
-cardnameW = HeapAlloc(GetProcessHeap(), 0, len * sizeof(WCHAR));
-if(!cardnameW){
-snd_ctl_close(ctl);
-return E_OUTOFMEMORY;
+cardnameW = szcardname;
}
-MultiByteToWideChar(CP_UNIXCP, 0, cardname, -1, cardnameW, len);
alsa_get_card_devices(stream, ids, keys, num, ctl, card, cardnameW);
-HeapFree(GetProcessHeap(), 0, cardnameW);
+if (cardnameW != szcardname)
+HeapFree(GetProcessHeap(), 0, cardnameW);
snd_ctl_close(ctl);
}
--
1.7.4.1
Re: [PATCH] winealsa: snd_card_get_name uses strdup (Valgrind).
Lauri Kenttä wrote: On 2012-02-01 17:00, Christian Costa wrote: In that case you don't need the if (!err) condition to free the buffer otherwise you leak the string return by strdup. Yeah, I didn't think this through and screwed up. :P I also noticed now that Jörg Höhle already deemed this strdup solution ugly in his previous try. Luckily Michael Stefaniuc sent a finer patch #83376. Still not good enough. But at least we forced Alexandre to fix it himself ;) bye michael
Re: mmdevapi: Correctly convert UINT32 to INT64
Hello Joerg, On 01/30/2012 09:46 AM, joerg-cyril.hoe...@t-systems.com wrote: thank you very much for finding a bug in my code. I'll have to meditate signed/unsigned conversions for the next months. Perhaps it's my lack of familiarity with the MS-Windows API that had me not find a signed equivalent of UINT32. Indeed there's none in windef.h. So I choose 'long'. But now I see basestd.h defines LONG32. LONG32 is kinda redundant. LONG is 32bit on all Windows versions on 16bit, 32bit and 64bit Windows. INT64 is not appropriate. getbuf_last should be of the same size than the other frame holding types. I once wrote a patch to turn all #frame holding entities from UINT64 into 32. Please rewrite the patch to use the LONG32 type, then you'll need no surprising cast. If the loss of 1 bit worries you, the solution IMHO is not to go to INT64 rather than prevent a duration that large in Initialize. In shared mode, there's no 2s limit. My tests opened a 10s buffer. I've not tested anything larger. LONG32 leaves enough room for 1 seconds at 20fps. bye michael
Re: comdlg32: Remove the font sample characters string.
Frédéric Delanoy wrote:
2012/1/18 Francois Gouget fgou...@free.fr:
On Wed, 18 Jan 2012, Frédéric Delanoy wrote:
On Wed, Jan 18, 2012 at 10:12, Francois Gouget fgou...@free.fr wrote:
The sample text does not depend on the dialog language but on the scripts
supported by the font. As such translating it in the resource file makes
no sense.
-CTEXT AaBbYyZz,stc5,103,80,109,24,SS_NOPREFIX | NOT
WS_VISIBLE
+CTEXT ,stc5,103,80,109,24,SS_NOPREFIX | NOT WS_VISIBLE
Actually it's used to show a sample in the font picker (e.g. with
wordpad do Format Font).
No it's not. The code overwrites this string when drawing the dialog and
then based on the entry selected in the 'Script:' combobox (cmb5). See
CFn_WMPaint():
if( GetWindowInfo( GetDlgItem( hDlg, stc5), info ) )
{
...
DrawTextW( hdc,
sample_lang_text[CHARSET_ORDER[lpcf-lpLogFont-lfCharSet]],
-1, info.rcWindow, DT_CENTER|DT_VCENTER|DT_SINGLELINE );
...
So the string which is in the resource file is useless and only causes
grief.
That's why I later added:
If the sample text does not depend on the dialogue language, that's
what should be done instead IMHO
i.e. alter the code so it does use the (translatable) string instead
of removing it altogether.
And what Francois is saying is that that isn't possible as the string
depends on the font and not on the UI language. So if I select a Klingon
font that has no latin characters I won't be able to see the AaBbYyZz.
bye
michael
Re: [PATCH 2/3] dsound: Merge IKsPropertySet into the secondary buffer object.
On 01/17/2012 05:44 PM, Andrew Eikum wrote: On Tue, Jan 17, 2012 at 01:53:10AM +0100, Michael Stefaniuc wrote: @@ -901,10 +895,11 @@ HRESULT IDirectSoundBufferImpl_Create( TRACE(Created buffer at %p\n, dsb); dsb-ref = 0; +dsb-refiks = 0; dsb-numIfaces = 0; dsb-device = device; dsb-IDirectSoundBuffer8_iface.lpVtbl = dsbvt; -dsb-iks = NULL; +dsb-IKsPropertySet_iface.lpVtbl = iksbvt; Mixed tabs and spaces :( Where do you see them? The lines that I added are purely spaces. dsound is mixed and inconsistent thus I use only spaces for the new lines. With the amount of churn we have there a lot of tabs should get cleaned up. bye michael
Re: [PATCH 5/8] dsound: Make capture behave like native in regards to COM aggregation.
joerg-cyril.hoe...@t-systems.com wrote:
Hi,
Michael Stefaniuc wrote:
static HRESULT WINAPI IDirectSoundCaptureImpl_CreateCaptureBuffer
+if (pUnk) {
+WARN(invalid parameter: pUnk != NULL\n);
+/* *lplpDSCaptureBuffer = NULL; Not done by native dsound */
+return DSERR_NOAGGREGATION;
+}
What's your rationale for doing so?
Bug compatibility with native is fine, however not setting NULL is
problematic before you've reached 100% compatibility, have you?
Not in this case which deals with COM aggregation. dsound doesn't
supports that and it is documented.
What I mean is that if there's a situation where Wine's CreateCaptureBuffer
returns some DSERR such as NOTIMPL where native does not, then that's an
unexpected situation for the app which it may not be well prepared to handle,
as it is untestable on native. Now not zeroing output pointers may make it
even harder for the app to remain in sane state.
Then blame the app or Wine for a crash?
If OTOH you know that the Wine module *always* returns the exact same
values as native, then I've no objection in not zeroing output pointers when
native does neither. Yet you'll have to consider that native may do so in
the future, witness a growing number of MSDN documents covering later
APIs that specifically mention zeroing pointers in case of failure.
IIRC, AJ removed from my mmdevapi tests one snippet that showed that native's
GetService
does not zero an output pointer even though MSDN says so -- Wine zeroes. Yet
perhaps
I got confused, messed up with patches and submitted the one that did not
contain that test
so maybe AJ never had a chance to see it. I'll have to check that.
Patch 8/8 has the tests for this. I used the standard COM aggregation
test for classes that do not support COM aggregation. That was designed
by Jacek in
http://source.winehq.org/git/wine.git/commitdiff/5b16e6e0fdb35e57a034fd5e6df61765ad11fa71
And the
ok(!unk, unk = %p\n, unk);
failed on the Win7 box I used for testing. That test is in because it is
known that Windows has classes that do not follow their own COM
standard, especially when it comes to COM aggregation.
Now i would have had different options how to deal with that.
- Silently drop the test.
- Dropping the test and documenting why it deviates from the standard test.
- Adding the test and marking it todo_wine.
- Add the test and fix the code as the change is trivial.
I picked IMHO the cleanest solution as that matches native 100% and adds
a test for that behavior.
bye
michael
Re: [PATCH 5/8] dsound: Make capture behave like native in regards to COM aggregation.
Hello Joerg,
and addendum inline
Michael Stefaniuc wrote:
joerg-cyril.hoe...@t-systems.com wrote:
Michael Stefaniuc wrote:
static HRESULT WINAPI IDirectSoundCaptureImpl_CreateCaptureBuffer
+if (pUnk) {
+WARN(invalid parameter: pUnk != NULL\n);
+/* *lplpDSCaptureBuffer = NULL; Not done by native dsound */
+return DSERR_NOAGGREGATION;
+}
What's your rationale for doing so?
Bug compatibility with native is fine, however not setting NULL is
problematic before you've reached 100% compatibility, have you?
Not in this case which deals with COM aggregation. dsound doesn't
supports that and it is documented.
What I mean is that if there's a situation where Wine's CreateCaptureBuffer
returns some DSERR such as NOTIMPL where native does not, then that's an
unexpected situation for the app which it may not be well prepared to handle,
as it is untestable on native. Now not zeroing output pointers may make it
even harder for the app to remain in sane state.
Then blame the app or Wine for a crash?
If OTOH you know that the Wine module *always* returns the exact same
values as native, then I've no objection in not zeroing output pointers when
native does neither. Yet you'll have to consider that native may do so in
the future, witness a growing number of MSDN documents covering later
APIs that specifically mention zeroing pointers in case of failure.
IIRC, AJ removed from my mmdevapi tests one snippet that showed that
native's GetService
does not zero an output pointer even though MSDN says so -- Wine zeroes. Yet
perhaps
I got confused, messed up with patches and submitted the one that did not
contain that test
so maybe AJ never had a chance to see it. I'll have to check that.
Patch 8/8 has the tests for this. I used the standard COM aggregation
test for classes that do not support COM aggregation. That was designed
by Jacek in
http://source.winehq.org/git/wine.git/commitdiff/5b16e6e0fdb35e57a034fd5e6df61765ad11fa71
And the
ok(!unk, unk = %p\n, unk);
Although I personally didn't hit that case yet my guess that check isn't
there just to detect the implementation details of the error handling.
It is there to detect if a class pretends to not support COM aggregation
but which still supports it. Just to give their own applications a
competitive advantage ...
failed on the Win7 box I used for testing. That test is in because it is
known that Windows has classes that do not follow their own COM
standard, especially when it comes to COM aggregation.
Now i would have had different options how to deal with that.
- Silently drop the test.
- Dropping the test and documenting why it deviates from the standard test.
- Adding the test and marking it todo_wine.
- Add the test and fix the code as the change is trivial.
I picked IMHO the cleanest solution as that matches native 100% and adds
a test for that behavior.
bye
michael
Re: [6/6] d3drm: Implement GetParent
On 01/09/2012 09:42 PM, André Hentschel wrote:
Am 08.01.2012 20:02, schrieb Nikolay Sivov:
On 1/8/2012 20:47, André Hentschel wrote:
---
dlls/d3drm/frame.c | 11 +++
dlls/d3drm/tests/d3drm.c | 16
2 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/dlls/d3drm/frame.c b/dlls/d3drm/frame.c
index 212f7dd..b72700c 100644
--- a/dlls/d3drm/frame.c
+++ b/dlls/d3drm/frame.c
@@ -33,6 +33,7 @@ typedef struct {
IDirect3DRMFrame2 IDirect3DRMFrame2_iface;
IDirect3DRMFrame3 IDirect3DRMFrame3_iface;
LONG ref;
+LPVOID parent;
} IDirect3DRMFrameImpl;
Parent is store as another frame interface pointer, right? Why void* here?
Didn't read that at first time. I wanted to access parent as
LPDIRECT3DRMFRAME, LPDIRECT3DRMFRAME2 and maybe LPDIRECT3DRMFRAME3
Store the object in the parent pointer and then you can easily return a
pointer to all interfaces implemented by the parent in a type safe manner:
IDirect3DRMFrameImpl *parent;
bye
michael
Re: Antw.: Remove obsolete and unused cmdlgtst.
On 12/08/2011 01:16 PM, Dmitry Timoshkov wrote: André Hentscheln...@dawncrow.de wrote: Yes, i just recently modified it to test a bug, please don't remove it You can always get through git when needed it. That or we split it out into a separate git tree. But we can keep it if it is still useful. My only problem with cmdltst is that it *wastes* a lot of translator time. If we keep it I'll send a patch to remove it from the translation status page. Alexandre already said that he won't convert it to PO files. bye michael
Re: dinput: Stub IDirectInputJoyConfig8 interface.
Hello Vitaliy, On 11/24/2011 06:28 PM, Vitaliy Margolen wrote: --- dlls/dinput/dinput_main.c|7 ++ dlls/dinput/dinput_private.h |2 + dlls/dinput/joystick.c | 195 + include/dinputd.h| 243 ++ 4 files changed, 447 insertions(+), 0 deletions(-) create mode 100644 include/dinputd.h we are trying to get rid off the LPJUNK, IDirectInputJoyConfig8 * is so much more nicer than LPDIRECTINPUTJOYCONFIG8. While the above is more of a nit-pick (and an easy search and replace patch for me) the QueryInterface implementation is not correct: - Query for IID_IUnknown is not supported. - Use IDirectInputJoyConfig8_AddRef() on the interface without the cast - You can save the round trip over impl_from_IDirectInputJoyConfig8() as iface and This-IDirectInputJoyConfig8_iface are the exact same thing. bye michael
Re: dinput: Stub IDirectInputJoyConfig8 interface.
On 11/24/2011 08:46 PM, Vitaliy Margolen wrote: On 11/24/2011 12:27 PM, Michael Stefaniuc wrote: Hello Vitaliy, On 11/24/2011 06:28 PM, Vitaliy Margolen wrote: --- dlls/dinput/dinput_main.c|7 ++ dlls/dinput/dinput_private.h |2 + dlls/dinput/joystick.c | 195 + include/dinputd.h| 243 ++ 4 files changed, 447 insertions(+), 0 deletions(-) create mode 100644 include/dinputd.h we are trying to get rid off the LPJUNK, IDirectInputJoyConfig8 * is so much more nicer than LPDIRECTINPUTJOYCONFIG8. IMHO - it's not. And it deviates from the PSDK headers, and the rest of the dinput. Not even saying that headers won't match declaration. I'm not talking about changing the headers but about the COM methods implementation. In addition to the usual complains about the LPJUNK typedefs in the case of COM interfaces there is also a loss of information as LPJUNK normally translates to JUNK*. bye michael
Re: user32/tests: Drop superfluous function pointer casts.
Marvin wrote: 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=15553 Your paranoid android. === W7PROX64 (64 bit msg) === Timeout === TEST64_W7SP1 (64 bit msg) === Timeout No clue why the final test result line is missing but a whitespace patch produces the exact same effect: https://testbot.winehq.org/JobDetails.pl?Key=1 bye michael
Re: qedit/tests: Add SampleGrabber COM aggregation test.
Marvin wrote: 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=15558 Your paranoid android. === WNT4WSSP6 (32 bit mediadet) === mediadet.c:147: Test failed: CoCreateInstance failed with 80040154 mediadet.c:148: Test failed: pM is NULL mediadet: unhandled exception c005 at 004015A2 === W2KPROSP4 (32 bit mediadet) === mediadet.c:147: Test failed: CoCreateInstance failed with 80040154 mediadet.c:148: Test failed: pM is NULL mediadet: unhandled exception c005 at 004015A2 === W2K8SE (32 bit mediadet) === mediadet.c:147: Test failed: CoCreateInstance failed with 80040154 mediadet.c:148: Test failed: pM is NULL mediadet: unhandled exception c005 at 004015A2 Those boxes don't have the qedit DLL. bye michael
Re: RFC: depending on getifaddrs in iphlpapi
Hello Juan, Juan Lang wrote: right now iphlpapi uses autoconf to detect if getifaddrs is available, and only uses it to enumerate IPv6 addresses. I'm thinking of replacing the current IPv4 enumeration code, which uses ioctl/SIOCGIFCONF, with getifaddrs. The reason I'm leaning toward replacing, rather than adding side-by-side with the current code, is that getifaddrs is commonly available: it's available on all recent versions of Linux, as well as on FreeBSD and MacOS. There recent versions of Linux as of which year? might be some platform somewhere that's disenfranchised by this, but I'm not sure it's worth the code complexity to maintain the current code. What about Solaris/OpenSolaris? That's the only other major system where Wine can be made to run. Any strenuous objections? Reasons to use getifaddrs rather than ioctls: 1. SIOCGIFCONF is actually pretty fiddly, and is the current code is broken on FreeBSD, or FreeBSD's support is broken (bug 27653.) 2. SIOCGIFFLAGS is apparently broken on current Linux versions. I haven't found any bug against Wine or Linux about that, I just noticed it while testing the existing code. bye michael
Re: cmd: Avoid mixing signed and unsigned type in conditional expression
On 10/27/2011 02:09 PM, Frédéric Delanoy wrote:
On Thu, Oct 27, 2011 at 08:38, Eric Pouech eric.pou...@orange.fr wrote:
why do we check for ptr being null or not, when we deref ptr one line above?
if (*ptr == '\n') ptr++;
- WCMD_output_asis_len(message, (ptr) ? ptr - message :
strlenW(message), handle);
+ WCMD_output_asis_len(message, (ptr) ? (DWORD)(ptr - message) :
strlenW(message), handle);
if (ptr) {
numChars = 0;
if (++line_count = max_height - 1) {
It's dereferenced but incremented afterwards, so might be null after that line
Only if ptr wraps around ;)
bye
michael
