Re: [PATCH 5/5] cmd: rename reference file from .out to .exp to avoid clash with gnu make builtin rule
On 02/16/2010 10:08 PM, Dan Kegel wrote: Without this, editing the .cmd file and then running make would cause make to overwrite the .out file with a copy of the .cmd file! Evidently '.out' is not appropriate as a suffix for a nongenerated source file. The arbitrarily-chosen .exp suffix works better (if anyone asks, it stands for 'expected output'). --- programs/cmd/tests/batch.c |4 ++-- programs/cmd/tests/rsrc.rc |4 ++-- programs/cmd/tests/test_builtins.cmd.exp | 18 ++ programs/cmd/tests/test_builtins.cmd.out | 18 -- 4 files changed, 22 insertions(+), 22 deletions(-) create mode 100644 programs/cmd/tests/test_builtins.cmd.exp delete mode 100644 programs/cmd/tests/test_builtins.cmd.out Hi Dan, These last changes introduce several test failures on some W2K, XP and W2K3 boxes but on all Wine ones: http://test.winehq.org/data/tests/cmd.exe:batch.html One thing I've noticed is that the succeeding ones run 12 tests whereas the failing ones run 14 tests. Could you have a look? -- Cheers, Paul.
Re: [PATCH 5/5] cmd: rename reference file from .out to .exp to avoid clash with gnu make builtin rule
On 02/18/2010 09:50 AM, Paul Vriens wrote: On 02/16/2010 10:08 PM, Dan Kegel wrote: Without this, editing the .cmd file and then running make would cause make to overwrite the .out file with a copy of the .cmd file! Evidently '.out' is not appropriate as a suffix for a nongenerated source file. The arbitrarily-chosen .exp suffix works better (if anyone asks, it stands for 'expected output'). --- programs/cmd/tests/batch.c | 4 ++-- programs/cmd/tests/rsrc.rc | 4 ++-- programs/cmd/tests/test_builtins.cmd.exp | 18 ++ programs/cmd/tests/test_builtins.cmd.out | 18 -- 4 files changed, 22 insertions(+), 22 deletions(-) create mode 100644 programs/cmd/tests/test_builtins.cmd.exp delete mode 100644 programs/cmd/tests/test_builtins.cmd.out Hi Dan, These last changes introduce several test failures on some W2K, XP and W2K3 boxes but on all Wine ones: http://test.winehq.org/data/tests/cmd.exe:batch.html One thing I've noticed is that the succeeding ones run 12 tests whereas the failing ones run 14 tests. Could you have a look? Nice, I just ran the latest tests again on winetestbot (as you also did before submitting) and they don't fail. So the big issue will be to reproduce these failures. I wasn't able on Windows, but I am on Wine when doing something like: wine winetest-latest.exe cmd.exe:batch Note that make batch.ok works just fine. I've build my own winetest and there I can trigger the failures on Wine as well. On to some debugging exercise. -- Cheers, Paul.
Re: [PATCH] wined3d: Split comments in separate line to avoid buffer overflow when traces are enabled (try 2)
On 17 February 2010 17:54, Christian Costa titan.co...@wanadoo.fr wrote:
+if (TRACE_ON(d3d_shader))
+{
+int size = strlen(comment) + 1;
+char* str = (char*)HeapAlloc(GetProcessHeap(), 0, size);
+int i = 0;
+char* line = str;
+memcpy(str, comment, size);
+DPRINTF(//);
+while (i size)
+{
+/* Find end of line */
+while ((str[i] != 0) (str[i] != 0x0a))
+i++;
+/* Terminate line and remove preceding 0x0d if any */
+if (i (str[i-1] == 0x0d))
+str[i-1] = 0;
+else
+str[i] = 0;
+/* Display line and prepare next line */
+DPRINTF(%s\n, debugstr_an(line, strlen(line)));
+i++;
+line = str + i;
+}
+}
This code has several obvious issues.
Re: [PATCH] wined3d: Split comments in separate line to avoid buffer overflow when traces are enabled (try 2)
Henri Verbeet a écrit :
On 17 February 2010 17:54, Christian Costa titan.co...@wanadoo.fr wrote:
+if (TRACE_ON(d3d_shader))
+{
+int size = strlen(comment) + 1;
+char* str = (char*)HeapAlloc(GetProcessHeap(), 0, size);
+int i = 0;
+char* line = str;
+memcpy(str, comment, size);
+DPRINTF(//);
+while (i size)
+{
+/* Find end of line */
+while ((str[i] != 0) (str[i] != 0x0a))
+i++;
+/* Terminate line and remove preceding 0x0d if any */
+if (i (str[i-1] == 0x0d))
+str[i-1] = 0;
+else
+str[i] = 0;
+/* Display line and prepare next line */
+DPRINTF(%s\n, debugstr_an(line, strlen(line)));
+i++;
+line = str + i;
+}
+}
This code has several obvious issues.
Right. HeapFree call and HeapAlloc failure handling are missing. I could
also do something with the leading // in
the case of an empty string (if this could ever happen).
If there are other obvious issues I've forgotten. Please be more accurate.
Re: [PATCH 5/5] cmd: rename reference file from .out to .exp to avoid clash with gnu make builtin rule
On 02/18/2010 11:36 AM, Paul Vriens wrote: On 02/18/2010 09:50 AM, Paul Vriens wrote: On 02/16/2010 10:08 PM, Dan Kegel wrote: Without this, editing the .cmd file and then running make would cause make to overwrite the .out file with a copy of the .cmd file! Evidently '.out' is not appropriate as a suffix for a nongenerated source file. The arbitrarily-chosen .exp suffix works better (if anyone asks, it stands for 'expected output'). --- programs/cmd/tests/batch.c | 4 ++-- programs/cmd/tests/rsrc.rc | 4 ++-- programs/cmd/tests/test_builtins.cmd.exp | 18 ++ programs/cmd/tests/test_builtins.cmd.out | 18 -- 4 files changed, 22 insertions(+), 22 deletions(-) create mode 100644 programs/cmd/tests/test_builtins.cmd.exp delete mode 100644 programs/cmd/tests/test_builtins.cmd.out Hi Dan, These last changes introduce several test failures on some W2K, XP and W2K3 boxes but on all Wine ones: http://test.winehq.org/data/tests/cmd.exe:batch.html One thing I've noticed is that the succeeding ones run 12 tests whereas the failing ones run 14 tests. Could you have a look? Nice, I just ran the latest tests again on winetestbot (as you also did before submitting) and they don't fail. So the big issue will be to reproduce these failures. I wasn't able on Windows, but I am on Wine when doing something like: wine winetest-latest.exe cmd.exe:batch Note that make batch.ok works just fine. I've build my own winetest and there I can trigger the failures on Wine as well. On to some debugging exercise. I think, I got it. It's a case-sensitivity issue. When I run the crosscompiled winetest on my Wine box I get for 'workdir': C:\users\paul\Temp\wct The test.out file however contains: C:\users\paul\temp\wct\ (notice the lower case 't' for temp). The same is probably true for several Windows boxes. On the winetestbot tests are run from the C:\winetest directory. Winetest.exe however uses the %TEMP%\wct directory which contains (XP/W2K3) something like 'Document and Settings'. -- Cheers, Paul.
Re: [PATCH 5/5] cmd: rename reference file from .out to .exp to avoid clash with gnu make builtin rule
On 02/18/2010 01:32 PM, Paul Vriens wrote: On 02/18/2010 11:36 AM, Paul Vriens wrote: On 02/18/2010 09:50 AM, Paul Vriens wrote: On 02/16/2010 10:08 PM, Dan Kegel wrote: Without this, editing the .cmd file and then running make would cause make to overwrite the .out file with a copy of the .cmd file! Evidently '.out' is not appropriate as a suffix for a nongenerated source file. The arbitrarily-chosen .exp suffix works better (if anyone asks, it stands for 'expected output'). --- programs/cmd/tests/batch.c | 4 ++-- programs/cmd/tests/rsrc.rc | 4 ++-- programs/cmd/tests/test_builtins.cmd.exp | 18 ++ programs/cmd/tests/test_builtins.cmd.out | 18 -- 4 files changed, 22 insertions(+), 22 deletions(-) create mode 100644 programs/cmd/tests/test_builtins.cmd.exp delete mode 100644 programs/cmd/tests/test_builtins.cmd.out Hi Dan, These last changes introduce several test failures on some W2K, XP and W2K3 boxes but on all Wine ones: http://test.winehq.org/data/tests/cmd.exe:batch.html One thing I've noticed is that the succeeding ones run 12 tests whereas the failing ones run 14 tests. Could you have a look? Nice, I just ran the latest tests again on winetestbot (as you also did before submitting) and they don't fail. So the big issue will be to reproduce these failures. I wasn't able on Windows, but I am on Wine when doing something like: wine winetest-latest.exe cmd.exe:batch Note that make batch.ok works just fine. I've build my own winetest and there I can trigger the failures on Wine as well. On to some debugging exercise. I think, I got it. It's a case-sensitivity issue. When I run the crosscompiled winetest on my Wine box I get for 'workdir': C:\users\paul\Temp\wct The test.out file however contains: C:\users\paul\temp\wct\ (notice the lower case 't' for temp). The same is probably true for several Windows boxes. On the winetestbot tests are run from the C:\winetest directory. Winetest.exe however uses the %TEMP%\wct directory which contains (XP/W2K3) something like 'Document and Settings'. Just sent a patch to fix the Wine failures. Forget the remarks about Windows and case-sensitivity, that needs more investigation. -- Cheers, Paul.
Re: [cmd] Don't change case of the batch filename
On 02/18/2010 02:35 PM, Paul Vriens wrote: Hi, This should fix the test failures for Wine (on test.winehq.org). Not sure how to write a (simple) test for this. The test is there in fact but it will only show up on test.winehq.org or when doing a make batch.ok from a directory with an uppercase letter in the path. Maybe we should create an uppercase directory in %TEMP% and run our tests from there instead of the current directory? Changelog Don't change case of the batch filename Ignore this one. Needs more thought. -- Cheers, Paul.
Re: [cmd] Don't change case of the batch filename
Paul Vriens paul.vriens.w...@gmail.com writes: diff --git a/programs/cmd/batch.c b/programs/cmd/batch.c index 28744d4..6cb732c 100644 --- a/programs/cmd/batch.c +++ b/programs/cmd/batch.c @@ -91,7 +91,7 @@ void WCMD_batch (WCHAR *file, WCHAR *command, int called, WCHAR *startLabel, HAN prev_context = context; context = LocalAlloc (LMEM_FIXED, sizeof (BATCH_CONTEXT)); context - h = h; - context-batchfileW = WCMD_strdupW(string); + context-batchfileW = WCMD_strdupW(file); That whole function is broken, it should not be lowering the string in the first place. It does this in order to use strstrW, but that's not the right way to check the extension anyway. In any case the test environment needs to be comparing paths in a case-insensitive way, it doesn't make sense to require exact case. -- Alexandre Julliard julli...@winehq.org
Re: [cmd] Don't change case of the batch filename
On 02/18/2010 02:51 PM, Alexandre Julliard wrote: Paul Vrienspaul.vriens.w...@gmail.com writes: diff --git a/programs/cmd/batch.c b/programs/cmd/batch.c index 28744d4..6cb732c 100644 --- a/programs/cmd/batch.c +++ b/programs/cmd/batch.c @@ -91,7 +91,7 @@ void WCMD_batch (WCHAR *file, WCHAR *command, int called, WCHAR *startLabel, HAN prev_context = context; context = LocalAlloc (LMEM_FIXED, sizeof (BATCH_CONTEXT)); context - h = h; - context-batchfileW = WCMD_strdupW(string); + context-batchfileW = WCMD_strdupW(file); That whole function is broken, it should not be lowering the string in the first place. It does this in order to use strstrW, but that's not the right way to check the extension anyway. I'm not even sure the extension checking is needed as the extension already seems to be present when calling WCMD_batch. In any case the test environment needs to be comparing paths in a case-insensitive way, it doesn't make sense to require exact case. So the memcmp needs to change into some strcmp form? -- Cheers, Paul.
Re: [cmd] Don't change case of the batch filename
Paul Vriens paul.vriens.w...@gmail.com writes: In any case the test environment needs to be comparing paths in a case-insensitive way, it doesn't make sense to require exact case. So the memcmp needs to change into some strcmp form? Yes, though you probably need CompareString. -- Alexandre Julliard julli...@winehq.org
Re: [cmd] Don't change case of the batch filename
On 02/18/2010 03:33 PM, Alexandre Julliard wrote:
Paul Vrienspaul.vriens.w...@gmail.com writes:
In any case the test environment needs to be comparing paths in a
case-insensitive way, it doesn't make sense to require exact case.
So the memcmp needs to change into some strcmp form?
Yes, though you probably need CompareString.
Could you think of a reason we need the extension checking in WCMD_batch() ?
I did some (small) tests and it seems like we could remove those checks
as WCMD_run_program() already seems to figure this out when it calls
WCMD_batch().
The attached patch gets rid of the checks and my tests still pass.
My test was wine cmd /c test.
Where test.cmd:
==
CALL test1
==
test1.cmd:
==
goto gotoreg
exit
:gotoreg
CALL :callreg
CALL :callreg
CALL test2
exit
:callreg
CALL reg
==
And test2.bat:
==
echo %~dp0
==
--
Cheers,
Paul.
diff --git a/programs/cmd/batch.c b/programs/cmd/batch.c
index 28744d4..9b78d81 100644
--- a/programs/cmd/batch.c
+++ b/programs/cmd/batch.c
@@ -47,35 +47,17 @@ extern DWORD errorlevel;
void WCMD_batch (WCHAR *file, WCHAR *command, int called, WCHAR *startLabel,
HANDLE pgmHandle) {
-#define WCMD_BATCH_EXT_SIZE 5
-
HANDLE h = INVALID_HANDLE_VALUE;
- WCHAR string[MAXSTRING];
- static const WCHAR extension_batch[][WCMD_BATCH_EXT_SIZE] =
{{'.','b','a','t','\0'},
-
{'.','c','m','d','\0'}};
- static const WCHAR extension_exe[WCMD_BATCH_EXT_SIZE] =
{'.','e','x','e','\0'};
- unsigned int i;
BATCH_CONTEXT *prev_context;
+ WINE_TRACE(file : (%s), command : (%s), startLabel : (%s)\n,
wine_dbgstr_w(file), wine_dbgstr_w(command), wine_dbgstr_w(startLabel));
+
if (startLabel == NULL) {
-for(i=0; (isizeof(extension_batch)/(WCMD_BATCH_EXT_SIZE * sizeof(WCHAR)))
- (h == INVALID_HANDLE_VALUE); i++) {
- strcpyW (string, file);
- CharLowerW (string);
- if (strstrW (string, extension_batch[i]) == NULL) strcatW (string,
extension_batch[i]);
- h = CreateFileW (string, GENERIC_READ, FILE_SHARE_READ,
- NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
-}
+h = CreateFileW (file, GENERIC_READ, FILE_SHARE_READ,
+NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
if (h == INVALID_HANDLE_VALUE) {
- strcpyW (string, file);
- CharLowerW (string);
- if (strstrW (string, extension_exe) == NULL) strcatW (string,
extension_exe);
- if (GetFileAttributesW (string) != INVALID_FILE_ATTRIBUTES) {
-WCMD_run_program (command, 0);
- } else {
-SetLastError (ERROR_FILE_NOT_FOUND);
-WCMD_print_error ();
- }
+ SetLastError (ERROR_FILE_NOT_FOUND);
+ WCMD_print_error ();
return;
}
} else {
@@ -91,7 +73,7 @@ void WCMD_batch (WCHAR *file, WCHAR *command, int called,
WCHAR *startLabel, HAN
prev_context = context;
context = LocalAlloc (LMEM_FIXED, sizeof (BATCH_CONTEXT));
context - h = h;
- context-batchfileW = WCMD_strdupW(string);
+ context-batchfileW = WCMD_strdupW(file);
context - command = command;
memset(context - shift_count, 0x00, sizeof(context - shift_count));
context - prev_context = prev_context;
Re: [cmd] Don't change case of the batch filename
Paul Vriens paul.vriens.w...@gmail.com writes: On 02/18/2010 03:33 PM, Alexandre Julliard wrote: Paul Vrienspaul.vriens.w...@gmail.com writes: In any case the test environment needs to be comparing paths in a case-insensitive way, it doesn't make sense to require exact case. So the memcmp needs to change into some strcmp form? Yes, though you probably need CompareString. Could you think of a reason we need the extension checking in WCMD_batch() ? I did some (small) tests and it seems like we could remove those checks as WCMD_run_program() already seems to figure this out when it calls WCMD_batch(). It looks like it's not needed. The case of calling a label in the same file would still set the wrong name, but not worse than it is now... -- Alexandre Julliard julli...@winehq.org
Re: comctl32: listview should accept both unicode and ansi notifications.
On 2/18/2010 15:46, Dmitry Timoshkov wrote: Listview receives notifications not only from built-in header control, but also from custom or subclassed application controls, there is no need to assert(0) on application input, printing a FIXME is the maximum we can do on an unknown input. The question actually is how does it react on it. Your test only check for return value and most likely doesn't use this conversion path (I'm not sure). The this is so strict probably was that we set notification format when header query for it, and we expect proper codes after that. Anyway change looks a bit confusing cause logically we shouldn't convert to ansi if we have ansi already. Added test case passes under XP, but listview in Wine intentionally crashes itself on an absolutely valid input. Yes, FIXME is ok here.
Re: [2/2] msi: Implement MSIRUNMODE_MAINTENANCE and MSIRUNMODE_REBOOTATEND for MsiGetMode.
On Thu, Feb 18, 2010 at 3:47 AM, Hans Leidekker h...@codeweavers.com wrote:
---
dlls/msi/install.c | 12 +++-
dlls/msi/tests/automation.c | 4 ++--
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/dlls/msi/install.c b/dlls/msi/install.c
index 7a71867..6e19863 100644
--- a/dlls/msi/install.c
+++ b/dlls/msi/install.c
@@ -661,6 +661,8 @@ BOOL WINAPI MsiGetMode(MSIHANDLE hInstall, MSIRUNMODE
iRunMode)
MSIPACKAGE *package;
BOOL r = FALSE;
+ TRACE(%d %d\n, hInstall, iRunMode);
+
package = msihandle2msiinfo(hInstall, MSIHANDLETYPE_PACKAGE);
if (!package)
{
@@ -706,8 +708,16 @@ BOOL WINAPI MsiGetMode(MSIHANDLE hInstall, MSIRUNMODE
iRunMode)
r = package-commit_action_running;
break;
+ case MSIRUNMODE_MAINTENANCE:
+ r = msi_get_property_int( package, szInstalled, 0 ) != 0;
+ break;
+
+ case MSIRUNMODE_REBOOTATEND:
+ r = package-need_reboot;
+ break;
+
default:
- FIXME(%d %d\n, hInstall, iRunMode);
+ FIXME(unimplemented run mode\n);
r = TRUE;
}
It's nice to see which run mode we're not handling by quickly looking
at the FIXME. Any reason you removed this? Keep in mind most general
user logs won't have +msi logging, if any.
--
James Hawkins
Re: [2/2] msi: Implement MSIRUNMODE_MAINTENANCE and MSIRUNMODE_REBOOTATEND for MsiGetMode.
On Thursday 18 February 2010 19:56:17 James Hawkins wrote: default: -FIXME(%d %d\n, hInstall, iRunMode); +FIXME(unimplemented run mode\n); r = TRUE; } It's nice to see which run mode we're not handling by quickly looking at the FIXME. Any reason you removed this? Keep in mind most general user logs won't have +msi logging, if any. That was unintentional, I'll send a fix. -Hans
Re: Is this a bug or feature incompleteness
On 02/18/2010 01:44 PM, MD.IMAM HOSSAIN wrote: The bug is from DirectX 8.1 SDK, C++ samples, Direct3D samples, SkinnedMesh. Please, see the attached file for clear idea. Tested with latest wine 1.1.38 Probably this: http://bugs.winehq.org/show_bug.cgi?id=6955 Missing vertex blending. -- Loïc Hoguin web: http://dev-extend.eu
Re: advapi32/tests: Avoid crashing if ReadEventLogA fails.
Hi Alexandre,
+if (ReadEventLogA(handle, EVENTLOG_SEQUENTIAL_READ |
EVENTLOG_FORWARDS_READ,
+ 0, buf, sizeof(EVENTLOGRECORD), read, needed))
+{
I don't think this is correct. The first call will always fail as the
buffer is not big enough. This now introduces a test failure on Vista
without a servicepack (what this piece of code was meant to check).
I guess a 'better' fix would have been to set needed to 0 in our
implementation until ReadEventLog is fully implemented.
--
Cheers,
Paul.
Re: Intercepting GDI calls
On Mon, Feb 15, 2010 at 4:05 PM, Ove Kaaven o...@arcticnet.no wrote: Sure it might be confusing, because that's not how the logic goes in the Microsoft world. Over there, the big machine acting as Terminal Server thing is the server, and the Remote Desktop client, which provides the actual display, is the client... while on X11, it's the complete opposite. I'm not going to pretend that it couldn't be confusing. The whole X11 client/server discussion was touched on in the Unix Haters Handbook chapter on X11. I think that reference proves the merit of the argument. -- Steven Edwards There is one thing stronger than all the armies in the world, and that is an idea whose time has come. - Victor Hugo
Re: Intercepting GDI calls
On 18 February 2010 22:08, Steven Edwards winehac...@gmail.com wrote: On Mon, Feb 15, 2010 at 4:05 PM, Ove Kaaven o...@arcticnet.no wrote: Sure it might be confusing, because that's not how the logic goes in the Microsoft world. Over there, the big machine acting as Terminal Server thing is the server, and the Remote Desktop client, which provides the actual display, is the client... while on X11, it's the complete opposite. I'm not going to pretend that it couldn't be confusing. The whole X11 client/server discussion was touched on in the Unix Haters Handbook chapter on X11. I think that reference proves the merit of the argument. http://uncyclopedia.wikia.com/wiki/X_Window_System ;-) - d.
Re: Intercepting GDI calls
Steven Edwards skrev: On Mon, Feb 15, 2010 at 4:05 PM, Ove Kaaven o...@arcticnet.no wrote: Sure it might be confusing, because that's not how the logic goes in the Microsoft world. Over there, the big machine acting as Terminal Server thing is the server, and the Remote Desktop client, which provides the actual display, is the client... while on X11, it's the complete opposite. I'm not going to pretend that it couldn't be confusing. The whole X11 client/server discussion was touched on in the Unix Haters Handbook chapter on X11. I think that reference proves the merit of the argument. Hmm. Well, I'll take your word for it, I haven't read that and don't plan to, since I'm quite happy with Unix and X11 myself. (Its logic also does make sense to me these days, though that doesn't mean I always understood its architecture. It looks like there's a learning curve to these things, which is why I know it could be confusing to people. Doesn't mean I don't like it. Often, I like confusing stuff.)
Multi-processor difference between Windows versions
In exploring a bug in a game we* ran across an interesting difference between how Windows versions handle a multiple-processor request. In essence, this supposedly invalid request will succeed on newer Windows versions **: SetThreadAffinityMask(curthread,(-1)); So, the question I have is whether Wine should support this behavior as a matter of course or if it should only support this request when the emulated version is set to Vista or greater. It seems to me like this should be supported as a matter of course, since anyone who uses this feature is unlikely to have properly tested it on multiple versions of Windows. However, I'm unfamiliar with the policy on these version-difference matters and figured I should ask before putting something together. Erich Hoover ehoo...@mines.edu * http://bugs.winehq.org/show_bug.cgi?id=21165 ** https://winetestbot.geldorp.nl/JobDetails.pl?Key=747
Re: Multi-processor difference between Windows versions
Hi Erich, On Thu, Feb 18, 2010 at 3:15 PM, Erich Hoover ehoo...@mines.edu wrote: In exploring a bug in a game we* ran across an interesting difference between how Windows versions handle a multiple-processor request. In essence, this supposedly invalid request will succeed on newer Windows versions **: SetThreadAffinityMask(curthread,(-1)); So, the question I have is whether Wine should support this behavior as a matter of course or if it should only support this request when the emulated version is set to Vista or greater. It seems to me like this should be supported as a matter of course, since anyone who uses this feature is unlikely to have properly tested it on multiple versions of Windows. However, I'm unfamiliar with the policy on these version-difference matters and figured I should ask before putting something together. You're right, we should do this as a matter of course. Usually the newer behavior is the saner behavior, so it's what we want to emulate. Please add a test case showing that SetThreadAffinityMask(curthread,(-1)) succeeds, and mark it as broken() if it doesn't. --Juan
Re: [PATCH 1/3] kernel32/tests: Add test for 'all processors' flag on Vista and newer.
On Thu, Feb 18, 2010 at 8:51 PM, Erich Hoover ehoo...@mines.edu wrote: Real Name: Erich Hoover Description: The attached patch adds a test for SetThreadAffinityMask(thread,-1), which succeeds on Windows Vista and newer. This all processors flag is not documented, but was discovered tracking down another issue. Please see the wine-devel thread for more information: http://www.winehq.org/pipermail/wine-devel/2010-February/081879.html ChangeLog: kernel32/tests: Add test for 'all processors' flag on Vista and newer. The test shouldn't query the Windows version explicitly. Instead, the older return value on pre-Vista version should be denoted by the broken() macro.
Re: [PATCH 1/3] kernel32/tests: Add test for 'all processors' flag on Vista and newer.
On Thu, Feb 18, 2010 at 8:29 PM, Andrew Nguyen arethus...@gmail.com wrote: On Thu, Feb 18, 2010 at 8:51 PM, Erich Hoover ehoo...@mines.edu wrote: ... The attached patch adds a test for SetThreadAffinityMask(thread,-1), which succeeds on Windows Vista and newer. ... The test shouldn't query the Windows version explicitly. Instead, the older return value on pre-Vista version should be denoted by the broken() macro. Then the test could only be triggered in Wine. What if this feature gets removed again in some later version of Windows? Erich Hoover ehoo...@mines.edu
Re: [PATCH 1/3] kernel32/tests: Add test for 'all processors' flag on Vista and newer.
Then the test could only be triggered in Wine. What if this feature gets removed again in some later version of Windows? broken() only applies to Windows versions, Wine never succeeds with a broken feature. It really is what you want. --Juan
Re: [PATCH 1/3] kernel32/tests: Add test for 'all processors' flag on Vista and newer.
On Thu, Feb 18, 2010 at 8:45 PM, Juan Lang juan.l...@gmail.com wrote: Then the test could only be triggered in Wine. What if this feature gets removed again in some later version of Windows? broken() only applies to Windows versions, Wine never succeeds with a broken feature. It really is what you want. --Juan I'm not arguing with the behavior of broken(). I'm saying that since this feature is undocumented it's entirely possible that it could get removed in some future version of Windows*, so if we're not testing the version we wouldn't know that it got removed. We were lucky to stumble upon this thing in the first place, it's starting to look like it's not actually the issue responsible for the slowdown bug for which we are researching. Erich Hoover ehoo...@mines.edu * Though I doubt it, as this seems like a sneaky way to make people believe that their multi-threaded apps run slower on XP,
Re: [PATCH 1/3] kernel32/tests: Add test for 'all processors' flag on Vista and newer.
I'm not arguing with the behavior of broken(). I'm saying that since this feature is undocumented it's entirely possible that it could get removed in some future version of Windows*, so if we're not testing the version we wouldn't know that it got removed. We were lucky to stumble upon this thing in the first place, it's starting to look like it's not actually the issue responsible for the slowdown bug for which we are researching. Except that an app already depends on the newer behavior, so Microsoft would have to maintain a quirk for the app if they wanted to change the behavior. Since the hypothetical future version of Windows with changed behavior doesn't yet exist, addressing that case seems rather premature. --Juan
Re: [PATCH 1/3] kernel32/tests: Add test for 'all processors' flag on Vista and newer.
On Thu, Feb 18, 2010 at 8:59 PM, Juan Lang juan.l...@gmail.com wrote: I'm not arguing with the behavior of broken(). I'm saying that since this feature is undocumented it's entirely possible that it could get removed in some future version of Windows*, so if we're not testing the version we wouldn't know that it got removed. We were lucky to stumble upon this thing in the first place, it's starting to look like it's not actually the issue responsible for the slowdown bug for which we are researching. Except that an app already depends on the newer behavior, so Microsoft would have to maintain a quirk for the app if they wanted to change the behavior. Since the hypothetical future version of Windows with changed behavior doesn't yet exist, addressing that case seems rather premature. --Juan Point taken. Erich Hoover ehoo...@mines.edu
Re: [PATCH 3/3] kernel32/tests: Test for 'all processors' now valid (try 2).
Erich Hoover wrote: Real Name: Erich Hoover Description: Patch 2 added support for the all processors flag, so this is no-longer a todo. This version is against the revised patch 1, please note that patch 2 is unchanged. ChangeLog: kernel32/tests: Test for 'all processors' now valid. Patches 2 and 3 should be merged. If patch 2 is applied without patch 3, then we'll get test succeeded inside todo block failures. Chip
