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

Re: [PATCH 1/2] dmime/tests: Added tests for IDirectMusicPerformance.

by Vitaliy Margolen

On 08/12/2010 12:09 AM, Austin Lund wrote: > On 12 August 2010 15:48, Vitaliy Margolen<wine-devel(a)kievinfo.com> wrote: >> You can't test that. All you can test is that you got the IDirectMusicPort >> back or not. What exactly inside is irrelevant. >> >> And if IDirectMusicPerformance8_PChannelInfo succeeds and you getting not >> null pDirectMusicPort back but it points to bogus data or is unchanged (as >> it is now), it's outright invalid behavior that should be fixed. No tests >> needed for that. > > OK. How about this: > > static void test_PChannelInfo(void) > { > IDirectMusicPort *pDirectMusicPort; > HRESULT hr; > > pDirectMusicPort = NULL; > hr = IDirectMusicPerformance8_PChannelInfo(idmusicperformance, 0, > &pDirectMusicPort, NULL, NULL); > ok(hr == S_OK, "Failed to call PChannelInfo (%x)\n", hr); > todo_wine ok(pDirectMusicPort != NULL, "IDirectMusicPort not set\n"); > } This looks much better. > I'll take on other comments and resubmit tomorrow as suggested. > +static void test_GetDefaultAudioPath(void) > +{ > + hr = IDirectMusicPerformance8_GetDefaultAudioPath(idmusicperformance,&pDirectMusicAudioPath); You leaking pDirectMusicAudioPath here. You should free it after test. Same for idmusicperformance. And CoInitialize() needs a matching CoUninitialize(). Vitaliy.

Aug. 12, 2010

Wine users with a CD-ROM drive, please test

by Joerg-Cyril.Hoehle＠t-systems.com

Hi, >mcicda.c:541: Test failed: status mode after play is playing >mcicda.c:548: Test failed: status mode after play is playing >mcicda: 105 tests executed (0 marked as todo, 2 failures), 1 skipped. Here I don't know what to do. Even after the former Sleep(3900ms), status mode reported playing. Perhaps some drives just enjoy spinning? If you look at the source, you'll see it does set c time format tmsf play c from 1 to 1:00:00:21 notify Sleep status c mode Within almost 4 seconds time, I'd have expect the drive to spin up, play for 280ms and stop again. I'm tempted to a) leave the test as is, so that test.winehq.org will slowly show how often this happens among users. Or do people think I should rather b) silence the error and don't care. After all, it tells me nothing about how to implement. BTW, in the next version, the 00:02:33 start position will not be flagged as error any more. Regards, Jörg Höhle

Aug. 12, 2010

Re: Fwd: Should we expect Liberation fonts to be installed?

by André Hentschel

Am 12.08.2010 06:11, schrieb Paul "TBBle" Hampson: > Sorry, I failed at Gmail again. >_< > > ---------- Forwarded message ---------- > From: Paul "TBBle" Hampson <Paul.Hampson(a)pobox.com> > Date: 12 August 2010 13:52 > Subject: Re: Should we expect Liberation fonts to be installed? > To: Scott Ritchie <scott(a)open-vote.org> > > > On 8 August 2010 13:02, Scott Ritchie <scott(a)open-vote.org> wrote: >> On 08/03/2010 01:57 PM, Scott Ritchie wrote: >>> I was looking through our fairly large collection of open font bugs and >>> realized that things might be a lot simpler if we took some opinionated >>> positions and just declared certain fonts to be dependencies and >>> expected all packagers to provide them. >>> >>> This is similar to bundling our own Tahoma, except much less work. >>> >>> >>> This bug, for instance, prevents Photoshop from working unless there is >>> an Arial font installed: http://bugs.winehq.org/show_bug.cgi?id=9623 >>> >>> Wine doesn't seem to respect system-level fontconfig aliases, so even >>> though Liberation Sans is installed on the system Photoshop won't try to >>> use it in place of Arial. >>> >>> But if however we assumed that Liberation Sans was installed, we could >>> make things much better: a link/substitution for Arial->Liberation Sans >>> could be provided in our own registry (and similarly for Times New Roman >>> and Courier). An alternative is to simply symlink to the Liberation >>> Fonts in /usr/share/wine/fonts as though they were our own shipped fonts >>> (like Tahoma). >>> >>> This would make Photoshop think Arial was present and keep it >>> functional. Ideally the real Arial would be displayed if it was >>> installed (eg through winetricks corefonts or by installing the >>> distro-provided corefonts package). >>> >>> >>> A related question is whether to show "Arial" in the list of fonts (eg >>> notepad) when we're actually just providing a substituted Arial. My >>> inclination says no, however I'm not sure how it works internally and >>> what an application would expect. >>> >> >> Assuming for a moment this is a good idea, what's the best >> implementation? My inclination is to say some registry font links, but >> I'm not completely familiar with how that works. >> >> Will font links in the registry be ignored when the real font is present? > > (Wine-specific) Font Replacements will be ignored when the real font > is present. Those're the ones in HKCU\Software\Wine\Fonts\Replacements > and probably the ones you want to use. (Which could be supplanted or > supplemented by working FontConfig alias support...) > > (GDI) Font Links (ie. the same registry entries Windows uses) are > fallbacks for missing glyphs. Those're the ones in > HKLM\SOFTWARE\Microsoft\FontLink\SystemLink > > Font Substitutes _probably_ don't get ignored if a real font with that > name exists. Those're the ones in HKLM\SOFTWARE\Microsoft\Windows > NT\CurrentVersion\FontSubstitutes and they're pretty purpose-specific. > The only entries we want for them in Wine are the ones described at > http://blogs.msdn.com/b/michkap/archive/2005/03/20/399322.aspx (which > should probably be the default in Wine anyway, at least the Tahoma > entry.) Wow, can you please update http://wiki.winehq.org/FontLoadOrder with these great informations? -- Best Regards, André Hentschel

Aug. 12, 2010

Re: shell32/tests: Test that a folder can be named when clicking "Make New Folder"

by Frank Richter

On 10.08.2010 20:22, Michael Mc Donnell wrote: > When a user clicks the "Make New Folder" button a new folder is created. > The name of the folder is selected, and the dialog box waits for the user > to either accept the name or type in a new one. The test types in a > new folder name and checks that the new folder gets that name. Also, you seem to emit “Alt+M” as part of the test. This is rather fragile, as the hotkey for “Make new folder” is most likely _not_ M on Windows or Wine translated to other than English. I believe the safe way to deal with that is to send a WM_COMMAND message with the ID of the “Make new folder” button. Wine test bot has a number of non-English VMs, so make sure you try your patches there. -f.r.

Aug. 12, 2010

CD-ROM ioctl support on MacOS?

by Joerg-Cyril.Hoehle＠t-systems.com

Hi, Charles Davis wrote: >> 1. READ_RAW >That's implemented. I implemented it myself in >6368dbbd5939bf71b04f3c441dd3af8cd027b7fb. It doesn't work here ("early 2009" mini with 10.5.8) as I mention in http://bugs.winehq.org/show_bug.cgi?id=20323#c4 Perhaps it can read data tracks only, no audio? Regards, Jörg Höhle

Aug. 12, 2010

Re: Re : [PATCH 1/2] dmime/tests: Added tests for IDirectMusicPerformance.

by Dmitry Timoshkov

paulo lesgaz <jeremielapuree(a)yahoo.fr> wrote: > Some compilers complain about the lack of a return in a void function. Don't > forget to add it in each void function. That compilers are buggy, there is no need to add statements without a real reason. At least it helps to list those compilers, and send a bug report if you think that's appropriate. -- Dmitry.

Aug. 12, 2010

Re : [PATCH 1/2] dmime/tests: Added tests for IDirectMusicPerformance.

by paulo lesgaz

Some compilers complain about the lack of a return in a void function. Don't forget to add it in each void function. A+ David ________________________________ De : Austin Lund <austin.lund(a)gmail.com> À : Vitaliy Margolen <wine-devel(a)kievinfo.com> Cc : wine-devel(a)winehq.org Envoyé le : Jeu 12 août 2010, 8h 09min 44s Objet : Re: [PATCH 1/2] dmime/tests: Added tests for IDirectMusicPerformance. On 12 August 2010 15:48, Vitaliy Margolen <wine-devel(a)kievinfo.com> wrote: > You can't test that. All you can test is that you got the IDirectMusicPort > back or not. What exactly inside is irrelevant. > > And if IDirectMusicPerformance8_PChannelInfo succeeds and you getting not > null pDirectMusicPort back but it points to bogus data or is unchanged (as > it is now), it's outright invalid behavior that should be fixed. No tests > needed for that. OK. How about this: static void test_PChannelInfo(void) { IDirectMusicPort *pDirectMusicPort; HRESULT hr; pDirectMusicPort = NULL; hr = IDirectMusicPerformance8_PChannelInfo(idmusicperformance, 0, &pDirectMusicPort, NULL, NULL); ok(hr == S_OK, "Failed to call PChannelInfo (%x)\n", hr); todo_wine ok(pDirectMusicPort != NULL, "IDirectMusicPort not set\n"); } I'll take on other comments and resubmit tomorrow as suggested. Thanks.

Aug. 12, 2010

Re: [PATCH 1/2] dmime/tests: Added tests for IDirectMusicPerformance.

by Austin Lund

On 12 August 2010 15:48, Vitaliy Margolen <wine-devel(a)kievinfo.com> wrote: > You can't test that. All you can test is that you got the IDirectMusicPort > back or not. What exactly inside is irrelevant. > > And if IDirectMusicPerformance8_PChannelInfo succeeds and you getting not > null pDirectMusicPort back but it points to bogus data or is unchanged (as > it is now), it's outright invalid behavior that should be fixed. No tests > needed for that. OK. How about this: static void test_PChannelInfo(void) { IDirectMusicPort *pDirectMusicPort; HRESULT hr; pDirectMusicPort = NULL; hr = IDirectMusicPerformance8_PChannelInfo(idmusicperformance, 0, &pDirectMusicPort, NULL, NULL); ok(hr == S_OK, "Failed to call PChannelInfo (%x)\n", hr); todo_wine ok(pDirectMusicPort != NULL, "IDirectMusicPort not set\n"); } I'll take on other comments and resubmit tomorrow as suggested. Thanks.

Aug. 12, 2010

Re: [PATCH 1/2] dmime/tests: Added tests for IDirectMusicPerformance.

by Vitaliy Margolen

On 08/11/2010 11:11 PM, Austin Lund wrote: > On 12 August 2010 14:23, Vitaliy Margolen<wine-devel(a)kievinfo.com> wrote: >> On 08/11/2010 09:14 PM, Austin Lund wrote: >>> >>> On 11 August 2010 20:12, David Adam<david.adam.cnrs(a)gmail.com> wrote: >>> I wasn't sure of the best way to do this. But basically I want to >>> check that the Vtbl had been set. I cannot cast to >>> IDirectMusicPortImpl and I'm still confused by the COM macros. >> >> You shouldn't be checking for any vtbl, it's all object's internal stuff. If >> you must check a pointer if it's null or not, then do that explicitly. >> Pointers have different size in 32-bit and 64-bit architectures. >> >> Also you don't need two things pointing to one another. IDirectMusicPort is >> a pointer in a first place. So no need for memset either. And no need for >> all the extra traces that print the same stuff. You already checking if it's >> null or not. The value of the pointer won't be of any use. > > The setting (or not) of the Vtbl is exactly why the crash in bug 22598 > occurs. So if I'm not allowed to test for this, then this bug > shouldn't have a test written for it? The IDirectMusicPort is a pointer. To zero it you do "ptr = NULL". Don't mention any vtbs in test. Think of it an (void*) pointer - what it points to shouldn't matter. > The test I wrote for this bug just ensures that the object has > something put into it to avoid the segfault. You can't test that. All you can test is that you got the IDirectMusicPort back or not. What exactly inside is irrelevant. And if IDirectMusicPerformance8_PChannelInfo succeeds and you getting not null pDirectMusicPort back but it points to bogus data or is unchanged (as it is now), it's outright invalid behavior that should be fixed. No tests needed for that. Vitaliy.

Aug. 12, 2010

Re: [PATCH 1/2] dmime/tests: Added tests for IDirectMusicPerformance. (try 2)

by Vitaliy Margolen

On 08/11/2010 11:16 PM, Austin Lund wrote: > On 12 August 2010 14:32, Vitaliy Margolen<wine-devel(a)kievinfo.com> wrote: >> >> What are those magic numbers? Please use constants. If they are missing, add >> them to appropriate header file first. > > They are the numbers passed to the InitAudio method from bug 22598. 1 is DMUS_APATH_SHARED_STEREOPLUSREVERB, 0x3f is DMUS_AUDIOF_ALL. > I've seen quite a few places where "magic" numbers are used in tests > with no references. I thought it would be ok to start the test off > this way. If they are a defined constant it's not a good thing to have them even in tests. Of course you won't have a constant for for things like 128 in your test (which is a number of performance channels). > What is the best way to resubmit for another round of review? To > wine-patches again (try 3)? Or on wine-devel? Or something else. Send to wine-patches, as long as you have all the problems fixed. Just wait a day to see if anyone else has any additional questions. Vitaliy.

Aug. 12, 2010

Re: [PATCH 1/2] dmime/tests: Added tests for IDirectMusicPerformance. (try 2)

by Austin Lund

On 12 August 2010 14:32, Vitaliy Margolen <wine-devel(a)kievinfo.com> wrote: > > What are those magic numbers? Please use constants. If they are missing, add > them to appropriate header file first. They are the numbers passed to the InitAudio method from bug 22598. I've seen quite a few places where "magic" numbers are used in tests with no references. I thought it would be ok to start the test off this way. What is the best way to resubmit for another round of review? To wine-patches again (try 3)? Or on wine-devel? Or something else.

Aug. 12, 2010

Re: [PATCH 1/2] dmime/tests: Added tests for IDirectMusicPerformance.

by Austin Lund

On 12 August 2010 14:23, Vitaliy Margolen <wine-devel(a)kievinfo.com> wrote: > On 08/11/2010 09:14 PM, Austin Lund wrote: >> >> On 11 August 2010 20:12, David Adam<david.adam.cnrs(a)gmail.com> wrote: >> I wasn't sure of the best way to do this. But basically I want to >> check that the Vtbl had been set. I cannot cast to >> IDirectMusicPortImpl and I'm still confused by the COM macros. > > You shouldn't be checking for any vtbl, it's all object's internal stuff. If > you must check a pointer if it's null or not, then do that explicitly. > Pointers have different size in 32-bit and 64-bit architectures. > > Also you don't need two things pointing to one another. IDirectMusicPort is > a pointer in a first place. So no need for memset either. And no need for > all the extra traces that print the same stuff. You already checking if it's > null or not. The value of the pointer won't be of any use. The setting (or not) of the Vtbl is exactly why the crash in bug 22598 occurs. So if I'm not allowed to test for this, then this bug shouldn't have a test written for it? The test I wrote for this bug just ensures that the object has something put into it to avoid the segfault. The sanest way I could think of doing this is to zero out the memory (hence memset) and check that the function actually put something at the start of the object. I got rid of the traces in the second go. They were left over from when I was using crosstest to see what was going on.

Aug. 12, 2010

Re: [PATCH 1/2] dmime/tests: Added tests for IDirectMusicPerformance. (try 2)

by Vitaliy Margolen

On 08/11/2010 08:58 PM, Austin Lund wrote: > > > +static BOOL test_InitAudio(void) > +{ > + hr = IDirectMusicPerformance8_InitAudio(idmusicperformance,NULL,&pDirectSound,NULL,1,128,0x3f,NULL); > + if (hr != S_OK) return FALSE; > + else return TRUE; > +} Please put spaces after comas, as you do in the rest of the file. What are those magic numbers? Please use constants. If they are missing, add them to appropriate header file first. Same thing with return as in prior function.. > +static void test_PChannelInfo(void) Already covered in another thread. Vitaliy.

Aug. 12, 2010

Re: [PATCH 1/2] dmime/tests: Added tests for IDirectMusicPerformance.

by Vitaliy Margolen

On 08/11/2010 09:14 PM, Austin Lund wrote: > On 11 August 2010 20:12, David Adam<david.adam.cnrs(a)gmail.com> wrote: > I wasn't sure of the best way to do this. But basically I want to > check that the Vtbl had been set. I cannot cast to > IDirectMusicPortImpl and I'm still confused by the COM macros. You shouldn't be checking for any vtbl, it's all object's internal stuff. If you must check a pointer if it's null or not, then do that explicitly. Pointers have different size in 32-bit and 64-bit architectures. Also you don't need two things pointing to one another. IDirectMusicPort is a pointer in a first place. So no need for memset either. And no need for all the extra traces that print the same stuff. You already checking if it's null or not. The value of the pointer won't be of any use. Vitaliy.

Aug. 12, 2010

Re: kernel32/tests: GetVolumeInformationA X: succeeds iff drive's default dir is root. (try 2)

by Vitaliy Margolen

On 08/11/2010 05:03 AM, Joerg-Cyril.Hoehle(a)t-systems.com wrote: > Hi, > > what happened to this patch? It's still marked new. > Is the discovery too surprising? > (Maybe it's the wrong time to ask with AJ still on vacation) According to this: http://www.winehq.org/pipermail/wine-devel/2010-August/085802.html you won't see any commits until Aug 16 at best. Vitaliy.

Aug. 12, 2010

Fwd: Should we expect Liberation fonts to be installed?

by Paul "TBBle" Hampson

Sorry, I failed at Gmail again. >_< ---------- Forwarded message ---------- From: Paul "TBBle" Hampson <Paul.Hampson(a)pobox.com> Date: 12 August 2010 13:52 Subject: Re: Should we expect Liberation fonts to be installed? To: Scott Ritchie <scott(a)open-vote.org> On 8 August 2010 13:02, Scott Ritchie <scott(a)open-vote.org> wrote: > On 08/03/2010 01:57 PM, Scott Ritchie wrote: >> I was looking through our fairly large collection of open font bugs and >> realized that things might be a lot simpler if we took some opinionated >> positions and just declared certain fonts to be dependencies and >> expected all packagers to provide them. >> >> This is similar to bundling our own Tahoma, except much less work. >> >> >> This bug, for instance, prevents Photoshop from working unless there is >> an Arial font installed: http://bugs.winehq.org/show_bug.cgi?id=9623 >> >> Wine doesn't seem to respect system-level fontconfig aliases, so even >> though Liberation Sans is installed on the system Photoshop won't try to >> use it in place of Arial. >> >> But if however we assumed that Liberation Sans was installed, we could >> make things much better: a link/substitution for Arial->Liberation Sans >> could be provided in our own registry (and similarly for Times New Roman >> and Courier). An alternative is to simply symlink to the Liberation >> Fonts in /usr/share/wine/fonts as though they were our own shipped fonts >> (like Tahoma). >> >> This would make Photoshop think Arial was present and keep it >> functional. Ideally the real Arial would be displayed if it was >> installed (eg through winetricks corefonts or by installing the >> distro-provided corefonts package). >> >> >> A related question is whether to show "Arial" in the list of fonts (eg >> notepad) when we're actually just providing a substituted Arial. My >> inclination says no, however I'm not sure how it works internally and >> what an application would expect. >> > > Assuming for a moment this is a good idea, what's the best > implementation? My inclination is to say some registry font links, but > I'm not completely familiar with how that works. > > Will font links in the registry be ignored when the real font is present? (Wine-specific) Font Replacements will be ignored when the real font is present. Those're the ones in HKCU\Software\Wine\Fonts\Replacements and probably the ones you want to use. (Which could be supplanted or supplemented by working FontConfig alias support...) (GDI) Font Links (ie. the same registry entries Windows uses) are fallbacks for missing glyphs. Those're the ones in HKLM\SOFTWARE\Microsoft\FontLink\SystemLink Font Substitutes _probably_ don't get ignored if a real font with that name exists. Those're the ones in HKLM\SOFTWARE\Microsoft\Windows NT\CurrentVersion\FontSubstitutes and they're pretty purpose-specific. The only entries we want for them in Wine are the ones described at http://blogs.msdn.com/b/michkap/archive/2005/03/20/399322.aspx (which should probably be the default in Wine anyway, at least the Tahoma entry.) (Which by-the-by highlights a mistake in my previous suggestion towards bug 13829) -- Paul "TBBle" Hampson, Paul.Hampson(a)Pobox.com On 12 August 2010 13:52, Paul "TBBle" Hampson <Paul.Hampson(a)pobox.com> wrote: > (Wine-specific) Font Replacements will be ignored when the real font > is present. Those're the ones in HKCU\Software\Wine\Fonts\Replacements > and probably the ones you want to use. (Which could be supplanted or > supplemented by working FontConfig alias support...) To be clear, I think the best first-step solution for this bug is for Wine's Font Replacements code to read the aliases from FontConfig as well as its own registry keys when building its list. (The latter being taken over the former) That would make the problem magically go away on systems that do include appropriately-substitutable fonts. -- Paul "TBBle" Hampson, Paul.Hampson(a)Pobox.com

Aug. 12, 2010

Re: [PATCH 1/2] dmime/tests: Added tests for IDirectMusicPerformance. (try 2)

by Austin Lund

On 12 August 2010 13:16, James McKenzie <jjmckenzie51(a)earthlink.net> wrote: > Austin Lund wrote: > > Why the try2? It is always nice to put why you are doing this in the git > commit notes. I was responding to review. I'm sorry if I've done something confusing.

Aug. 12, 2010

Re: [PATCH 1/2] dmime/tests: Added tests for IDirectMusicPerformance. (try 2)

by James McKenzie

Austin Lund wrote: Why the try2? It is always nice to put why you are doing this in the git commit notes. AJ is on vakay and will not be back until next week so there are NO reviews ongoing this week, except developer peer ones. James McKenzie

Aug. 12, 2010

Re: [PATCH 1/2] dmime/tests: Added tests for IDirectMusicPerformance.

by Austin Lund

On 11 August 2010 20:12, David Adam <david.adam.cnrs(a)gmail.com> wrote: > Hello, thanks for the comments. I've resent. > + todo_wine ok(*(ULONG *)pDirectMusicPort != 0, "IDirectMusicPort not > set\n"); > > Why is this cast useful? I wasn't sure of the best way to do this. But basically I want to check that the Vtbl had been set. I cannot cast to IDirectMusicPortImpl and I'm still confused by the COM macros.

Aug. 12, 2010

Re: [(try 2) 02/11] d3d10: Implement D3D10CreateBlob().

by Stefan Dösinger

Am 11.08.2010 um 21:51 schrieb Rico Schüller: > --- a/dlls/d3d10/d3d10_private.h > +++ b/dlls/d3d10/d3d10_private.h > @@ -212,6 +212,17 @@ struct d3d10_effect > struct d3d10_effect_technique *techniques; > }; > > > +extern const struct ID3D10BlobVtbl d3d10_blob_vtbl DECLSPEC_HIDDEN; Do you need the declaration of the vtable in the header? As far as I can see you're not using it anywhere outside d3d10_main.c.

Aug. 11, 2010

Re: [PATCH] qmgr/tests: Detect, when the service is disabled

by testbot＠testbot.winehq.org

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=4405 Your paranoid android. === W98SE (32 bit qmgr) === qmgr.c:46: Test failed: CoCreateInstance failed: 80040154

Aug. 11, 2010

Re: Wine users with a CD-ROM drive, please test

by Erik Inge Bolsø

On Wed, 11 Aug 2010, Joerg-Cyril.Hoehle(a)t-systems.com wrote: > Hi, > > testbot.winehq.org job #4404 now contains another patch and binary. I removed the call to > set c door closed > estimating that this was the most likely among the three suspects to cause the error > (I also reduced the Sleep() time). Yay, no more MCIERR_HARDWARE! Some others, though. Win7 64bit. Z:\test>mcicda_test_4404.exe mcicda.c:287: CD length 60:42:06 mcicda.c:351: track #1 length 245334ms mcicda.c:423: Tests skipped: Got no mixed data+audio CD. mcicda.c:441: last track length: 06m:46s:25frames mcicda.c:449: last track position: 53m:57s:56frames mcicda.c:541: Test failed: status mode after play is playing mcicda.c:548: Test failed: status mode after play is playing mcicda: 105 tests executed (0 marked as todo, 2 failures), 1 skipped. > Please test yet once again. I've not changed anything about the 00:02:33 start position > which I'll leave in until testbot gathers more data from users. I had that one too, for one out of five test cds or so. -- -erik http://useofwords.blogspot.com/

Aug. 11, 2010

Re: [PATCH 1/2] dmime/tests: Added tests for IDirectMusicPerformance.

by Chris Robinson

On Wednesday, August 11, 2010 3:12:23 am David Adam wrote: > Hello, > > some comments about this patch > + if (hr != S_OK) return FALSE; > + else return TRUE; > > could be simplified as > return SUCCEEDED(hr); SUCCEEDED encompasses more than just S_OK (S_FALSE, for instance; basically anything that doesn't have the high bit set). Whether to test SUCCEEDED or S_OK explicitly depends on what the function can return and what it means. The original behavior could instead be simplified as: return hr==S_OK;

Aug. 11, 2010

Wine users with a CD-ROM drive, please test

by Joerg-Cyril.Hoehle＠t-systems.com

Hi, testbot.winehq.org job #4404 now contains another patch and binary. I removed the call to set c door closed estimating that this was the most likely among the three suspects to cause the error (I also reduced the Sleep() time). Please test yet once again. I've not changed anything about the 00:02:33 start position which I'll leave in until testbot gathers more data from users. Thank you very much, Jörg Höhle

Aug. 11, 2010

kernel32/tests: GetVolumeInformationA X: succeeds iff drive's default dir is root. (try 2)

by Joerg-Cyril.Hoehle＠t-systems.com

Hi, what happened to this patch? It's still marked new. Is the discovery too surprising? (Maybe it's the wrong time to ask with AJ still on vacation) Is it not robust enough (lacking testing with network drives)? It explains bugs #20887 and #23355. It may explain intermittent failures in test.winehq/kernel/volume.ok, as success depends on where the test was launched from. >Tests pass on w95,w98,w2k,xp,7. I forgot to add "without skipping the one relevant section", i.e. when started from another drive than C: Of course I invite people here to also perform the test on native. Regards, Jörg Höhle

Aug. 11, 2010