I've sent a patch set that I need some help testing. The first four in the series should be fine as they don't change anything specific to a particular platform. The last one however makes some changes which are platform specific (affects Linux, Apple, and various BSDs). I've tested the Linux code but I do not have an Apple, for example, to test that code. While the code looks good to me and is is largely based off of code removed from fill_cpu_info(), it is still worth some concern that it hasn't seen any real testing. If you have a Mac or BSD, I would be grateful if you could try the patches out to make sure things are working as expected. You'll find a few more details in the commit message http://source.winehq.org/patches/data/89698 .
Also of concern is that the testbot seems to have only one processor. This results in most of the tests added in http://source.winehq.org/patches/data/89697 being skipped. So testbot doesn't seem that helpful for this test.
As for the implementation of the test itself, would it be better to use broken() and just test Wine for the Win7 behavior? From my testing, the test as is works for XP, Win7, and Wine with my patch set. If we can write the test to work everywhere, don't use broken()? There are more details in the patch itself so please have a look and let me know what you think.
-- Jim
Hi James,
On Sep 10, 2012, at 10:27 AM, James Eder wrote:
I've sent a patch set that I need some help testing. The first four in the series should be fine as they don't change anything specific to a particular platform. The last one however makes some changes which are platform specific (affects Linux, Apple, and various BSDs). I've tested the Linux code but I do not have an Apple, for example, to test that code. While the code looks good to me and is is largely based off of code removed from fill_cpu_info(), it is still worth some concern that it hasn't seen any real testing. If you have a Mac or BSD, I would be grateful if you could try the patches out to make sure things are working as expected. You'll find a few more details in the commit message http://source.winehq.org/patches/data/89698 .
Thanks for this work.
First, there are some compiler warnings:
gcc -m32 -c -I. -I. -I../../include -I../../include -D__WINESRC__ -D_NTSYSTEM_ -D_REENTRANT -fPIC -Wall -pipe -fno-strict-aliasing -Wdeclaration-after-statement -Wempty-body -Wstrict-prototypes -Wwrite-strings -fno-omit-frame-pointer -Wpointer-arith -I/usr/X11/include/freetype2 -I/usr/X11/include -g -gstabs -o nt.o nt.c nt.c: In function ‘fill_cpu_info’: nt.c:1207: warning: unused variable ‘longVal’ nt.c: In function ‘NtPowerInformation’: nt.c:2470: warning: format ‘%lu’ expects type ‘long unsigned int’, but argument 6 has type ‘ULONG’ nt.c:2470: warning: format ‘%lu’ expects type ‘long unsigned int’, but argument 7 has type ‘ULONG’ nt.c:2470: warning: format ‘%lu’ expects type ‘long unsigned int’, but argument 8 has type ‘ULONG’ nt.c:2470: warning: format ‘%lu’ expects type ‘long unsigned int’, but argument 9 has type ‘ULONG’ nt.c:2470: warning: format ‘%lu’ expects type ‘long unsigned int’, but argument 10 has type ‘ULONG’ nt.c:2470: warning: format ‘%lu’ expects type ‘long unsigned int’, but argument 11 has type ‘ULONG’
Then, you are off by an order of magnitude. You're dividing by 10 million rather than 1 million to go from Hz to MHz.
After I fixed that, it gave:
trace:ntdll:NtPowerInformation (11,0x0,0,0x1105f0,96) warn:ntdll:NtPowerInformation semi-stub: ProcessorInformation trace:ntdll:NtPowerInformation cpu_power[0] = 0 2800 2800 2800 0 0 trace:ntdll:NtPowerInformation cpu_power[1] = 1 2800 2800 2800 0 0 trace:ntdll:NtPowerInformation cpu_power[2] = 2 2800 2800 2800 0 0 trace:ntdll:NtPowerInformation cpu_power[3] = 3 2800 2800 2800 0 0
My machine is indeed a quad-core, 2.8 Ghz Mac Pro.
I would say you should probably remove that semi-stub warning.
Cheers, Ken
On Mon, Sep 10, 2012 at 11:16 PM, Ken Thomases ken@codeweavers.com wrote:
First, there are some compiler warnings:
Fixed
Then, you are off by an order of magnitude. You're dividing by 10 million rather than 1 million to go from Hz to MHz.
and fixed
I would say you should probably remove that semi-stub warning.
Doh! I sent without that one. I'll remove the warning and send again probably tomorrow or the next day. I'll wait a bit to see if we get any comments from the BSD side. I am downloading FreeBSD for testing in a QEMU but I'm not sure how long it will take for me to get it up and rolling.
Thanks Ken!