-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
This is my first attempt at contributing anything to the Wine tree, so if I've made any common 'newbie' mistakes, that's probably why.
I've been experimenting with dx9wine to see if I can get Planetside working (that game is the only reason I'd even consider going back to windows), but the program requires one function from powrprof.dll, which, unless I'm blind, doesn't seem to exist in the wine tree at present. So I spent a few hours looking at the source tree and looking up info on the functions provided by powrprof.dll, and wrote up a header and a .c file that includes stubbed out functions so that applications that use powrprof will at least load (they of course might not run properly). Attached you'll find the result of that work, including a patch to configure.ac and dlls/Makefile.in, and a tarball containing dlls/powrprof/ and include/powrprof.h. Note that powrprof.h contains several struct and enum definitions that really belong in winnt.h, but I wasn't sure where in winnt.h I should place them, so I simply stuck them in powrprof.h for now.
After compiling powrprof.dll.so and placing it in /usr/lib/dx9wine/lib/wine/, I was able to get the Planetside exe to load, but it promptly bombed out because it thought my graphics card was only 16 megs (it's 128, but I'm sure I just need to fix a config variable), so I think I'm on the right track.
Thank you for your time.
Better patch, plus I messed up one function name (leave it to MS to break their own convention in their own library).
Hi Benjamin,
Benjamin Cutler wrote:
Better patch, plus I messed up one function name (leave it to MS to break their own convention in their own library).
Generally looks good. I'd advise you to try get the header modifications in first in a seperate patch, then send a different patch for the implementation of the DLL.
I'll add some comments:
--- wine/configure 2005-04-09 21:33:30.000000000 -0600 +++ wine.new/configure 2005-04-09 21:32:53.000000000 -0600
You don't need to diff configure, only configure.ac
diff -urN wine/dlls/Makefile.in wine.new/dlls/Makefile.in
You can probably omit dlls/Makefile.in too, as it is also automatically generated (by make_dlls).
diff -urN wine/include/powrprof.h wine.new/include/powrprof.h --- wine/include/powrprof.h 1969-12-31 17:00:00.000000000 -0700 +++ wine.new/include/powrprof.h 2005-04-09 20:07:39.000000000 -0600
+#ifndef _POWRPROF_H +#define _POWRPROF_H 1
Most of the Wine headers use the prefix __WINE_ -> __WINE_POWRPROF_H
+#include <stdarg.h>
+#include "windef.h" +#include "winbase.h" +#include "winnt.h" +#include "winreg.h" +#include "winternl.h"
Don't include other headers in your header unless the Windows Platform SDK does it (it doesn't in this case). We would like the header dependencies to be the same as the Platform SDK, so programs can compile the same way.
+/* Constants needed by various functions */
+/* Used by SYSTEM_POWER_LEVEL structures */
+#define DISCHARGE_POLICY_CRITICAL 0 /* When battery hits critical threshold */ +#define DISCHARGE_POLICY_LOW 1 /* When battery hits low threshold */
These are defined in winnt.h. You need to patch that, not add it to your new file. You shouldn't add comments for all these things unless there's some trick to them.
+/* FIXME: These enums belong in winnt.h */
Yes. Is there any reason you didn't fix it?
So have a go at patching all the headers first, and then after the header patches are accepted, try submitting the dll implementation. Smaller patches are easier to get accepted.
Mike
Mike McCormack wrote:
You don't need to diff configure, only configure.ac
You can probably omit dlls/Makefile.in too, as it is also automatically generated (by make_dlls).
Alright, I'll take these out of the next diff.
Most of the Wine headers use the prefix __WINE_ -> __WINE_POWRPROF_H
Fixed.
Don't include other headers in your header unless the Windows Platform SDK does it (it doesn't in this case). We would like the header dependencies to be the same as the Platform SDK, so programs can compile the same way.
Hmm, ok. I'll have to make some changes to the .c file then. Thanks for pointing this out.
These are defined in winnt.h. You need to patch that, not add it to your new file. You shouldn't add comments for all these things unless there's some trick to them.
+/* FIXME: These enums belong in winnt.h */
Yes. Is there any reason you didn't fix it?
So have a go at patching all the headers first, and then after the header patches are accepted, try submitting the dll implementation. Smaller patches are easier to get accepted.
Mike
Is there some way that winnt.h is organized, or can I just throw them in at the end? That header file is gigantic so I can't tell if it's supposed to be organized in some particular way.
Benjamin Cutler wrote:
Is there some way that winnt.h is organized, or can I just throw them in at the end? That header file is gigantic so I can't tell if it's supposed to be organized in some particular way.
Function calls are at the end. Groups of enums and defines that relate to the same kind of stuff are together, more or less...
Just add it somewhere where it looks good to you ... probably in the middle somewhere after other groups of enums.
Mike
Mike McCormack wrote:
Function calls are at the end. Groups of enums and defines that relate to the same kind of stuff are together, more or less...
Just add it somewhere where it looks good to you ... probably in the middle somewhere after other groups of enums.
Mike
Hmm, I glanced through it, and it looked to me like sticking it at the end was as good as anywhere else, and I didn't have to worry about falling into some #ifdef block I hadn't noticed, so that's where I put it.
Better patch.
Benjamin Cutler wrote:
Hmm, I glanced through it, and it looked to me like sticking it at the end was as good as anywhere else, and I didn't have to worry about falling into some #ifdef block I hadn't noticed, so that's where I put it.
Actually, there's already a power management section in there, albeit a very small one. It's on line 3383 of winnt.h in the CVS. Maybe stick it in there?
I would still recommend submitting two seperate patches to wine-patches. One for headers, and one for the dll itself.
You might consider adding a .cvsignore file to your dll patch too.
Mike
On Sat, 9 Apr 2005, Benjamin Cutler wrote: [...]
Don't include other headers in your header unless the Windows Platform SDK does it (it doesn't in this case). We would like the header dependencies to be the same as the Platform SDK, so programs can compile the same way.
Hmm, ok. I'll have to make some changes to the .c file then. Thanks for pointing this out.
Let me explain this rule a bit more just for the benefit of the general public:
There's one important and slightly counterintuitive property that we want in Wine: we want programs to *fail to compile* in Wine the same way they fail to compile in Windows.
The rationale is that programs developped in Wine should have a reasonable chance to compile and link on Windows using the Platform SDK.
This is especially important for the conformance tests where typically one developer writes a conformance test using Wine's headers and then another developer compiles it on Windows using the Platform SDK. At some point most conformance tests would just fail to compile on Windows because the Wine headers were more permissive for the #include order, C COM interface declarations, etc. This meant more work for the second developer and conformance tests were not being run much on Windows thus defeating their purpose. But I'm happy to report that now things are much better.
-
Benjamin Cutler -
Francois Gouget -
Mike McCormack