January 2010 - Wine-Devel - list.winehq.org

Re: [PATCH] Revert "dsound: Only initialize one guid for capture and renderer."

by Alexandre Julliard

Maarten Lankhorst <m.b.lankhorst(a)gmail.com> writes: > This reverts commit b474649e0e9491f938f1daec74cdca01343dd97e. Please explain why. -- Alexandre Julliard julliard(a)winehq.org

Jan. 4, 2010

Re: Patch feedback requested for OleCreatePropertyFrame()

by Huw Davies

On Sun, Jan 03, 2010 at 03:00:12PM -0800, Geoffrey Hausheer wrote: > I found a patch from 2001 written by TAKESHIMA Hidenori that > was posted to wine-patches > (http://www.winehq.org/pipermail/wine-patches/2001-October/001229.html) > but never committed. Actually it was commited (2843934af5515c7f2b8370324aa98d3964a40324) but was then removed (140e7222e0d7ce76068cddc64c68105c2e569257) at his request. Huw.

Jan. 4, 2010

Re: Patch feedback requested for OleCreatePropertyFrame()

by Maarten Lankhorst

Hi Geoffrey, 2010/1/4 Geoffrey Hausheer <winedevel9605(a)phracturedblue.com>: > Some software I'm using is configured via an OleCreatePropertyFrame > call which isn't implemented in Wine (ticket #16564). I can use > winetricks + dcom98 to get it working, but would prefer not to need > that. I found a patch from 2001 written by TAKESHIMA Hidenori that > was posted to wine-patches > (http://www.winehq.org/pipermail/wine-patches/2001-October/001229.html) > but never committed. I cleaned up the patch to compile against > current Git and am including it here for inspection. I can't find any > documentation on why it wasn't included at that time, and haven't done > any Wine development for a long time, so I'm not sure what it will > take to get it included. > > I have tested it, and it works pretty much fine for me (not quite as > pretty as the dcom menu, but it is functional). I'd be happy to write > a test for it if I had any idea how to do that, but I'm not sure where > to begin for a function like this. > > Anyhow, I'm open to feedback on what should be done before I submit > the patch for inclusion > > FYI: I was unable to find a currently valid email address for Takeshima Those defines bother me: +#define CPropertyPageContainerImpl_AddRef(pContainer) (++((pContainer)->ref)) +#define CPropertyPageContainerImpl_Release(pContainer) (--((pContainer)->ref)) Interlocked* functions? The other define looks ugly, testcases are missing, and A functions are used, rather than the W functions, which is probably what blocked this. Some of the FIXME messages should really be warns too. Cheers, Maarten.

Jan. 4, 2010

Patch feedback requested for OleCreatePropertyFrame()

by Geoffrey Hausheer

Some software I'm using is configured via an OleCreatePropertyFrame call which isn't implemented in Wine (ticket #16564). I can use winetricks + dcom98 to get it working, but would prefer not to need that. I found a patch from 2001 written by TAKESHIMA Hidenori that was posted to wine-patches (http://www.winehq.org/pipermail/wine-patches/2001-October/001229.html) but never committed. I cleaned up the patch to compile against current Git and am including it here for inspection. I can't find any documentation on why it wasn't included at that time, and haven't done any Wine development for a long time, so I'm not sure what it will take to get it included. I have tested it, and it works pretty much fine for me (not quite as pretty as the dcom menu, but it is functional). I'd be happy to write a test for it if I had any idea how to do that, but I'm not sure where to begin for a function like this. Anyhow, I'm open to feedback on what should be done before I submit the patch for inclusion FYI: I was unable to find a currently valid email address for Takeshima .Geoff

Jan. 3, 2010

Re: Use GCC's -Wlogical-op if possible

by Gerald Pfeifer

On Sun, 3 Jan 2010, Austin English wrote: >> On my FreeBSD test system I am see no warnings triggered by -Wlogical-op >> any more. How does it look on your side? > ole32: > usrmarshal.c:485: warning: logical ?&&? with non-zero constant will > always evaluate as true > usrmarshal.c:1098: warning: logical ?&&? with non-zero constant will > always evaluate as true > usrmarshal.c:1290: warning: logical ?&&? with non-zero constant will > always evaluate as true > > oleaut32: > variant.c:2090: warning: logical ?||? with non-zero constant will > always evaluate as true > variant.c:2090: warning: logical ?&&? with non-zero constant will > always evaluate as true > > comctl32/tests: > tab.c:520: warning: logical ?&&? with non-zero constant will always > evaluate as true > tab.c:540: warning: logical ?&&? with non-zero constant will always > evaluate as true > tab.c:563: warning: logical ?&&? with non-zero constant will always > evaluate as true > tab.c:978: warning: logical ?&&? with non-zero constant will always > evaluate as true I had a patch for this one (comctl32/tests) which I received feedback on and need to brush up. I should be able to do so coming week. Anybody volunteering to look into the other ones? > kernel32/tests: > atom.c:70: warning: logical ?||? with non-zero constant will always > evaluate as true Gerald

Jan. 3, 2010

Re: Use GCC's -Wlogical-op if possible

by Austin English

On Sun, Jan 3, 2010 at 4:13 AM, Gerald Pfeifer <gerald(a)pfeifer.com> wrote: > On Tue, 23 Jun 2009, Paul Vriens wrote: >>> For those interested, here's the make log in Monday's git. 94 warnings >>> for me. >> Sent 3 patches based on your log. >> >> I have less (55) warnings on 4.3.2 btw, but most look like false positives. > > On my FreeBSD test system I am see no warnings triggered by -Wlogical-op > any more. How does it look on your side? ole32: usrmarshal.c:485: warning: logical ‘&&’ with non-zero constant will always evaluate as true usrmarshal.c:1098: warning: logical ‘&&’ with non-zero constant will always evaluate as true usrmarshal.c:1290: warning: logical ‘&&’ with non-zero constant will always evaluate as true oleaut32: variant.c:2090: warning: logical ‘||’ with non-zero constant will always evaluate as true variant.c:2090: warning: logical ‘&&’ with non-zero constant will always evaluate as true comctl32/tests: tab.c:520: warning: logical ‘&&’ with non-zero constant will always evaluate as true tab.c:540: warning: logical ‘&&’ with non-zero constant will always evaluate as true tab.c:563: warning: logical ‘&&’ with non-zero constant will always evaluate as true tab.c:978: warning: logical ‘&&’ with non-zero constant will always evaluate as true kernel32/tests: atom.c:70: warning: logical ‘||’ with non-zero constant will always evaluate as true -- -Austin

Jan. 3, 2010

Re: [1/7] avifile: Initial test data and simple test for avifil32 depends

by Michael Stefaniuc

On 01/01/2010 10:11 PM, Austin English wrote: > On Fri, Jan 1, 2010 at 2:18 PM, Julius Schwartzenberg > <julius.schwartzenberg(a)gmail.com> wrote: >> I think it was also stupid of me to send all the patches as a serie, while >> they do not actually depend on each other. I'm not sure though how I would >> submit the patches next time. Should I send the tests depending on each >> other in a separate serie from the actual fixes or is the right way actually >> to send a fix with it's test in the same serie? > > It's better to send the test in first, in case the fix isn't > committed. As for the series, if the patches don't depend on each > other, there's no need to make a series, though it doesn't hurt > anything. It does hurt to make independent patches into a patch series: if Alexandre rejects one patch in the series that will block the rest of the patches. bye michael

Jan. 3, 2010

Re: Use GCC's -Wlogical-op if possible

by Gerald Pfeifer

On Tue, 23 Jun 2009, Paul Vriens wrote: >> For those interested, here's the make log in Monday's git. 94 warnings >> for me. > Sent 3 patches based on your log. > > I have less (55) warnings on 4.3.2 btw, but most look like false positives. On my FreeBSD test system I am see no warnings triggered by -Wlogical-op any more. How does it look on your side? Depending on your findings, we may want to enable -Wlogical-op by default and I'd submit a patch to that end. Gerald

Jan. 3, 2010

Re: comctl32: cchTextMax needs to be set for LVS_OWNERDATA as it uses LVN_GETDISPINFO notifications

by Jeff Latimer

On 03/01/10 08:29, Jeff Latimer wrote: > On 03/01/10 01:12, Nikolay Sivov wrote: >> On 1/2/2010 13:52, Jeff Latimer wrote: >>> On 02/01/10 21:34, Nikolay Sivov wrote: >>>> On 1/2/2010 12:48, Jeff Latimer wrote: >>>>> --- >>>>> dlls/comctl32/listview.c | 1 + >>>>> 1 files changed, 1 insertions(+), 0 deletions(-) >>>> >>>>> + dispInfo.item.cchTextMax = textlenT(pszText, isW)+2; >>>> What does this '2' mean? >>> +1 adds the trailing null and +2 allows for the original text to be >>> greater than the edited text. >> I'm ok with +1 probably, but what about comparison? Why can't it be >> greater without this change? >>> I thought that one extra byte would allow the test to complete in >>> all cases. >> Don't know about tests, but it looks like you're hiding a real >> problem with this change. > I not sure what you mean here. My thinking is that the existing text > is X characters long. The new text will be Y. If X is greater that Y > (ie. the new text has had the trailing characters deleted) then all I > need is to prove that there is one extra character in the existing > text to continue through to the update. Given this, I don't need to > go to the expense of calculating the exact value of X as it is not > necessary. Rereading MSDN, +1 will probably work ok. If the buffer is short then the characters copied will not contain the terminating null, hence will not match but one won't be a string. Will develop a test.

Jan. 2, 2010

Re: comctl32: cchTextMax needs to be set for LVS_OWNERDATA as it uses LVN_GETDISPINFO notifications

by Reece Dunn

2010/1/2 Nikolay Sivov <bunglehead(a)gmail.com>: > On 1/2/2010 13:56, Jeff Latimer wrote: >> >> On 02/01/10 21:42, Nikolay Sivov wrote: >>> >>> On 1/2/2010 12:48, Jeff Latimer wrote: >>>> >>>> --- >>>> dlls/comctl32/listview.c | 1 + >>>> 1 files changed, 1 insertions(+), 0 deletions(-) >>> >>> Actually setting buffer count is noop here, cause buffer pointer is not >>> set up for dispInfo.item. >> >> Because it is tied up with callbacks, I have observed that the callback >> routine in the app may be the issue here. It looks to be used in at least >> one case. > > It's tied of course, but such partial change isn't clear, if you have a real > problem with that, please file a bug with application or test affected. >From MSDN (http://msdn.microsoft.com/en-us/library/bb774760%28VS.85%29.aspx) cchTextMax is the maximum number of TCHARs (including NULL) that can be stored in pszText. That is, it supports things like: TCHAR buffer[100]; item.pszText = buffer; item.cchTextMax = sizeof(buffer) / sizeof(TCHAR); That link says that cchTextMax is read-only during LVN_GETDISPINFO and that pszText points to a buffer that can hold cchTextMax TCHARs, but in the LVN_GETDISPINFO example (http://msdn.microsoft.com/en-us/library/bb774818%28VS.85%29.aspx) it uses assignment instead of copy, i.e.: item.pszText = "hello"; instead of: strncpy(item.pszText, "hello", item.cchTextMax); >>> A test required for this - let's say check that an ownerdata listview >>> send LVN_GETDISPINFO before text update and the same thing for callback text >>> (don't remember now maybe similar tests are already in). >>> >> I can do a test but not for 2 weeks (summer holidays). > > Summer? Ok. Southern hemisphere. They get to avoid the snow and cold weather. - Reece

Jan. 2, 2010