Contents 1 Coding Hints 1.1 General Information 1.1.1 Mac 1.1.2 Editor Settings 1.1.2.1 Emacs 1.1.2.2 Vim 1.1.3 Scripts 1.1.4 Operator precedence 1.1.5 Case Statements 1.1.6 Commenting Code 1.1.6.1 Referring to Bugzilla 1.1.7 Copyright and License Code Comments 1.1.8 Commenting out Code 1.2 Code Readability suggestions and opinions 1.2.1 Hungarian Notation 1.2.2 Tabs 1.3 Configure 1.3.1 pck-config 1.4 Make File 1.4.1 Delay-load 1.5 Winecfg 1.6 wine.inf 1.7 Spec. Files 1.7.1 arch 1.8 Big Endian vs Little 1.9 Headers 1.9.1 Matching PSDK 1.9.2 Exported Headers 1.9.2.1 Undocumented API 1.9.3 #ifdef 1.9.4 #include 1.9.5 #Define 1.9.5.1 L Defines 1.9.5.2 Definitions which conflict with unix 1.9.5.3 Define Magic Numbers 1.9.5.4 Define Naming Styles 1.9.5.5 WIN32_LEAN_AND_MEAN 1.9.5.6 Macros 1.9.5.6.1 When to use a Function 1.9.5.6.2 INVALID_FILE_SIZE 1.9.5.6.3 ERROR_SUCCESS 1.9.5.6.4 PathAddBackslash 1.9.6 Declaring Variables 1.10 Declaring and Renaming Variables 1.10.1 Declaring 1.10.2 Meaningful names 1.10.3 Renaming Variables 1.10.4 .tlb files 1.11 Windows Compatability 1.11.1 Wine32 strings 1.11.2 Long 1.11.3 Win64 compatability

Coding Hints

Sending patches to most open source projects can be challenging for anyone new to the project. By looking over previous patch reviews, you can learn from the developers suggestions. Read through the following for many tips and suggestions for sending patches to wine. You can list further examples here. Keep in mind that opinions change and wine is a fast moving project. If you spot an error, you are invited to help fix it, but please add the date so in future people can bear that in mind.

Where to start if you are new to wine?
A programmer noted: I am of course unfamiliar with the whole wine source code. I did look over the rather daunting to-do list and wouldn't even know where to start. =)

D.Kegel [May 2007 wine devel]: I would suggest starting by doing a couple weeks of bug triage. See http://kegel.com/wine/qa/ Then, while still doing bug triage, start writing conformance tests to expose some of the bugs you're seeing. Getting conformance tests committed to the WineHQ tree is a good rite of passage. Then, once you have the hang of that, pick one of the bugs you've written a good conformance test for, and try to fix it.

D. Kegel [Jun 07 wine devel] recommended reading the official Coding Hints documenation. See http://winehq.org/site/docs/winedev-guide/codingpractice [...] Be sure to include a conformance test!

Other pages of interest may be:

• Coding Hints:Using Diff
• Coding Hints:Test Cases
• Coding Hints:Patches Submitting Patches
• Want_to_be_a_Wine_Coder?

General Information

LIST_FOR_EACH( cursor, list)
{
   TYPE list_entry = LIST_ENTRY( cursor, TYPE, entry );
   ...

R. Shearman [May 05]: Can be replaced by the following for IMHO, slightly more readable code: Wine Archives

LIST_FOR_EACH_ENTRY( list_entry, list, TYPE, entry)
{
   ...

Mac

There has been some talk about working with code for the mac, however J. McKenzie [feb 10] There can be no code that cannot compile or be ignored on other *NIX platforms. S. Dosinger: Alexandre has said that you can put objective C code into wine, but only if this code is properly abstracted from the other parts of the quartz driver. E.g. by a simple binding wrapper that does nothing but wrap Objective C to C bindings. The requirement is that somebody can read and understand the Quartz driver without knowing Objective C.

The biggest obstacle to merging a Quartz driver is cleaning up the user32 driver interface. Objective C is a minor issue with a pretty straightforward solution. Currently nobody knows what a proper user32 driver interface would look like.

Editor Settings

Emacs

Are there Editor settings (e.g. emacs c-mode) resulting in acceptable indentation?

A. Juliard: I'm using the "stroustrup" style in Emacs. Also make sure to set indent-tabs-mode to nil.

J. Zaroyko [feb 10]: In emacs c-mode, I find M-x c-toggle-electric-state is the best way to not interfere with the surrounding indentation.

Vim

W. Sang: http://wiki.winehq.org/HackingTips Has some useful bits for vim

Scripts

A programmer asked [jun 05]: Do I need to do anything special for a configure.ac patch - does Alexandre re-run autoconf on commit, or do I need to do that and include the patch to configure too?

J. Lang: I've seen patches both ways. Try it and see :) Wine Archive

It was suggested to use bash features in wine shell scripts.

S. Gouget: Some users may not have bash installed and the script should still work on their systems [...]Wine should not depend on bash.

D. Kegel: Unless you have a *very* good reason,you should stick to Posix interfaces, which here means "get it to run under Bourne shell". Portability -- it's a good thing. By the way, if anyone out there is looking for a concise, illuminating guide to portable shell scripting, I highly recomment "Portable Shell Programming" by Bruce Blinn.Wine Archive Link

Operator precedence

-    if ((width >= 0x10000) || (height >= 0x10000))
+    if ((height && width >= 0x10000) || (width && height >= 0x10000))
{
        FIXME("got bad width %d or height %d, please look for reason\n",width, height);

Height && width evaluantes to either 0 or 1, same with width && height, so will the fixme ever be produced?

R. Klazes pointed out: Operator ">=" has precedence over "&&", so the evaluation is as: if ((height && (width >= 0x10000)) || (width && (height >= 0x10000))) Wine Archive Link

dlls/advapi32/crypt.c :1649
 
 if (!key || !pbData || !key->pProvider || key->pProvider->dwMagic != MAGIC_CRYPTPROV)
     CRYPT_ReturnLastError(ERROR_INVALID_PARAMETER);
 
 [...]

A. Julliard [May 05]: The && and || operators in C are short-circuit operators, meaning they are always evaluated left to right, and further tests are not executed if the first ones fail.Wine Archive Link

+ if (!((LISTVIEW_GetItemState(infoPtr, infoPtr->nFocusedItem, uMask) & uMask) == uMask)) return -1;

D. Timoshkov [May 05] Wine Archives: Wouldn't it be more readable and less confusing in the [this] form?
if ((LISTVIEW_GetItemState(infoPtr, infoPtr->nFocusedItem, uMask) & uMask) != uMask) return -1;

Case Statements

         case WM_COMMAND:
-            if (HIWORD(wParam) == EN_CHANGE)
+            switch (HIWORD(wParam))
             {

M. McCormack: [May 05] I don't know if it's just my opinion, but case statements within case statements make the code more difficult to read. Would you consider creating a new function instead? Wine Archives eg.

case WM_COMMAND:

OnCommand( dialog, HIWORD(wParam), LOWORD(wParam) );

break;

+        case WB_STOP:
+            IWebBrowser2_Stop(pWebBrowser2);
+    }

---

D. Timoshkov [July 05]: [using] default: here wouldn't hurt as well and would show some not expected/implemented cases with a FIXME message. Wine Archive

Commenting Code

What to comment upon M. McCormack [May 06]: My pet hate comments are stuff like:

/* My tests reveal that it's done this way */

ofcourse, those tests aren't in the test suite.

/* FIXME: this is broken */

With no explanation of why.

J. White: Yes, obviously written code to perform an obvious function needs few, if any, comments. And I think I would agree that the Wine server is commented about right; it is, imho, a beautiful piece of code. [...]

D. Riekenberg: Many of my Patches add the missing WineAPI - Documentation, but the overall API Documentation - Status is still to low.

M. Bestefich: The purpose of comments is to let strangers get approximately acquainted with how the code works in abstract, what the call structure is, etc. - NOT to document what individual lines of code do.

U. Bonnes quoted: "It's painfully having to figure out what exactly a function does, that calls five other functions that you have to look into, which each call another cryptically named function". and then added: Also, sometimes it's not obvious why a function is called. A comment like "Call funcResetStatusVariables because the above code triggers a redraw which can break them" would save a new developer twenty minutes of "But why update them THERE?! Does my code have to do that?".

Commenting code example D. Timoshkov [Aug 06] [reviewed the following code and commented: I'd suggest to move GetTextMetricsW outside of the loop to not kill the performance.

+DWORD WineEngGetGlyphIndices(HDC hdc, GdiFont font, LPCWSTR lpstr, INT count,LPWORD pgi, DWORD flags)
{
    INT i;
-
+    TEXTMETRICW textm;
    for(i = 0; i < count; i++)
+    {
        pgi[i] = get_glyph_index(font, lpstr[i]);
-
+        if  (pgi[i] == 0)
+        {
+            if  (flags & GGI_MARK_NONEXISTING_GLYPHS) + pgi[i] = 0x001f; /* Indicate non existance */
+            else
+            {
+                GetTextMetricsW(hdc, &textm);
+                pgi[i] = textm.tmDefaultChar; 
+            }
+        }
+    }
    return count;
}
}

The programmer commented: I put it inside the loop as I assumed that a non existent glyph would be relatively rare and the call would not happen much. This seemed preferable to doing the call every time the function was called.

A. Mohr: This should have gone into a comment right there because it's a very normal reaction to immediately question code like that, so the code should properly defend itself by default ;) IOW just the usual "do coding as obvious as possible, then properly comment everything else that isn't obvious". Maybe something like wine archive

/* called in a loop, but missing glyph shouldn't happen often
 so we don't want to call it outside the loop, always */

Referring to Bugzilla

A. Julliard: I'm not opposed to having [a bugzilla] bug number in the comments, but it has to be in addition to a proper explanation; the log must be understandable on its own without making reference to an external database. [Ed. link is to beginning of thread] wine archive

---

Segin [Apr 06] Please replace your "// comment" style comments with "/* comment */" ones wine archive

//    ULONG ref;

We avoid C++ comments in Wine code to keep compatability with older compilers.

+//#include <wchar.h>

R. Shearman: Remove this line [of dead code].

+/* if (dwPropID != AMPROPERTY_PIN_CATEGORY)
+      return E_PROP_ID_UNSUPPORTED;

R. Shearman:Any reason why this is commented out? Leaving things like this in the source tends to introduce an element of doubt when someone next looks at the code.

Changed file overview:
- shell32_?.rc: Comment out two items (and a separator) and copy text from IDS_SELECT, 
needed to match the current context menu

A. Julliard: You shouldn't comment things out like that. Either the info is needed and it should be here, or it's not needed and it should be removed completely. Commenting it out is only creating confusion Wine Archive.

R. Shearman [Nov 05] highlights a factor to consider when commenting code: c2man compatible documentation format [is desired].wine archive

Copyright and License Code Comments

LGPL

+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA

J. Hawkins [Jun 06]: The address has changed, and we recently did a sweep of the code base changing to the new address, so we'd like to keep the old address out. wine archive

* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301,  USA

>+/*
>+ * Implementation of the Local Security Authority API
>+ *
>+ * This library is free software; you can redistribute it and/or
>+ * modify it under the terms of the GNU Lesser General Public
>+ * License as published by the Free Software Foundation; either
>+ * version 2.1 of the License, or (at your option) any later version.
>+ *
>+ * This library is distributed in the hope that it will be useful,
>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>+ * Lesser General Public License for more details.
>+ *
>+ * You should have received a copy of the GNU Lesser General Public
>+ * License along with this library; if not, write to the Free Software
>+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301,  USA
>+ */

R. Shearman: You should include a copyright line that states the major copyright owners of the code. You can do this by looking through the CVS history or just by copying the copyright lines from security.c Wine Archive Ed [Jun 06]. ammended address to 51 Franklin st.

[July 05] M. McCormack: Add an LGPL copyright header to the .c and .h files

Copyright

+ * Copyright 2005

M. McCormack: You need to add your name after the copyright.

+/* Copyright 2005 [name redacted] Goal is to get MSN's webcam function to work..

M. McCormack: Better to put the description on one line first, and the Copyright message below it, as described in the COPYING file on line 473.

Commenting out Code

+    /* Crashes on Win2K
     st = pAcquireCredentialsHandleA(NULL, unisp_name_a, 0, NULL, NULL, NULL, NULL, NULL, NULL);
     ok(st == SEC_E_NO_CREDENTIALS, "Expected SEC_E_NO_CREDENTIALS, got %08x\n", st);
+     */

>

J. Hawkins suggested [Oct 06]: Please use #if 0/#endif to comment out pieces of code.

M. McCormack: Or even better use if (0) { } so compile problems don't creep in...wine archive

Code Readability suggestions and opinions

Hungarian Notation

A. Julliard seems to dislike it.

   if (wFlags & DISPATCH_PROPERTYGET) {
                 V_VT(pVarResult) = VT_I4;
-                V_I4(pVarResult) = data->iCount; 
+                V_I4(pVarResult) = data->ulCount;

A. Julliard [May 07]: I committed this patch, but I can't resist pointing out that if you had simply named the field "count" instead of using the (idiotic) Hungarian notation, the code would be just as clear, and you wouldn't need to rename it everywhere when making it unsigned...

The Developer queried: is the general recommendation for wine that we not use Hungarian notation ?

Stefan: Alexandre usually calls it Hungarian Line Noise, I guess that means "no".

Unusually he did say [Jan 09 wine dev] when reviewing the following patch From: H Verbeet:

+0100 Subject: ddraw: Rename the main vtables to just "vtbl".

A. Julliard: I hate to suggest adding Hungarian notation noise, but "lpVtbl" may be a better choice since that's the standard name in all COM macros and generated code. It would make it more obvious that this is the regular main vtable.

A. Talbot: IMHO, a prefix is of little value if it merely echos the declared type of the variable it represents, especially in a small-scope situation. To be of value, a prefix needs to convey other information, such as implying the purpose of the variable, or acting as a grouping element for a set of related variables. [wine devel May 20 07]

Tabs

D. Riekenberg [apr 09] : Wine use[s] 4 SPACE for indention. There is old code that use[s] 2 or 3 SPACE, some old locations use TAB (width 8) and also some locations with TAB and SPACE mixed can be found. The rule is to stay with the same style as used in the file before (2, 3 or 4 SPACE), but never mix SPACE and TAB in touched lines. http://permalink.gmane.org/gmane.comp.emulators.wine.devel/68548

Changing only the indention is not desired, as the history is falsified (Original Author and date/time of the the last code-change) [C. Costa pointed out this affects what shows in git blame]. Expect patches with unrelated indention changes to be rejected. I suggest to ask Julliard [about patches to convert to 4 spaces] before you start with that. He might exceptional accept that style changes only Patch, as you did the major part of the code in that dll.

For new files, there is no old style to cover with, so 4 SPACE indention is expected.

+       notify_listview(infoPtr, LVN_BEGINDRAG, &nmlv);
+
+       return 0;
+    }

Dimi Paun [may05]: Please use 4-space indentation for new code. Currently listview is a mixture of 2/4 space indentation, but we should be moving towards 4 spaces if it's not a problem Wine Archive Link.

A user asked: I've always wondered, why not tabs? Any decent text editor can display tabs however wide someone wants them to be. Wouldn't that solve the whole problem? Or is there some other problem with tabs that I'm missing?

Dimi Paun[Jun 05]: In fact, if editors wouldn't have allowed such a thing, we would be in a much saner state now. Why not tabs:

• the standard is 8
• many common editors do not support tab redefinition (e.g. notepad)
• tools assume 8 (cvsweb, viewcvs, etc)
• printers always print it as 8
• you don't want to setup your editor before actually doing work
• if people mix tabs and spaces (and they do) the output is not sane

[...]In the Wine tree we currently use 2, 4, 8 space indents. [For those who feel strongly about their favourite spacing] I [am] sure this is diverse enough that you can pick one of the above and be (reasonably) happy with it. Wine Archive

A Programmer noted that some of original source code does not have 4 space indents and asked if he should follow this with newer code.

Dimi Paun [May05]:it's best if new, reviewed code use 4-space indents, so we can separate it easily from the older code Wine Archives.

Configure

D. Riekenberg:[Apr 06] When I send a Patch, that result in a change for "configure", I just add a note in the Mail:The Changes for "configure" are not included. Alexandre rebuilds "configure", when needed wine archive

+ configure.ac 4 Jul 2007 15:45:29 -0000

D, Riekenberg: Please do not include any autogenerated code. The files are updated by "tools/make_makefiles", when Alexandre commit your Patch. This prevents conflicting Patches, when another Program is added at the same Time.

S. Dosinger [Nov 08]: Don't modify ./configure directly, edit configure.ac instead. The autoconf documentation describes how this file works. configure is then created from configure.ac by running "autoconf"

F.Gouget: configure is really not a file that anyone would want to modify manually. It may be regenerated alone from time to time to update it to newer versions of autoconf, but it's definitely not modified manually.

J. Lang replied to the question: Isn't all the configuration done via ./tools/make_makefiles ? I thought that if you want to add anything to wine you:

1. simply add it to git
2. ./tools/make_makefiles
3. autoconf
4. ./configure make depend && make

[...]

No, not all.[...] On occasion we need to add or improve checks for different libraries. tools/make_makefiles is used to add new code to Wine, configure.ac is modified to add new configure checks. The use of configure.ac is well-described on the GNU autoconf page.

D. Riekenberg: When a new directory was added to Wine (git-update-index --add */Makefile.in), then configure.ac is updated by make_makefiles The Makefile has a rule to rebuild configure, when configure.ac changed, The Makefile has a rule to rerun config.status, when configure changed (That will rerun configure / rebuild the Makefiles) So in the line 3 above is only present to reduce the total time.

pck-config

A programmer asked [jan 09]: I would like to know if there is any issues as to why we are not using pkg-config to check for needed library's with the m4 macros.

A. Julliard: We use it in a few places where we have no choice, but in general it's doing more harm than good. The whole concept is broken.

Make File

Someone realized [may 2005] that we can't compile man pages outside the source directory, c2man complains about being unable to find .c files and asked how this should be fixed. Because documentation/man3w should be in the build directory, we could modify TOPSRCDIR and TOPOBJDIR for the makefiles[...] to:

TOPSRCDIR = @abs_top_srcdir@
TOPOBJDIR = @abs_top_builddir@

A. Julliard: No, you don't want to put absolute paths in makefiles. c2man should be fixed to load files from the source directory if they are not found in the build directory. It makes it impossible to copy or move build trees around, [and] that's very annoying. Wine Archive Link

Resource files

diff -urw ../wine/dlls/urlmon/Makefile.in dlls/urlmon/Makefile.in
--- ../wine/dlls/urlmon/Makefile.in	2005-09-06 19:18:47.000000000 +0200
+++ dlls/urlmon/Makefile.in	2005-09-03 21:05:48.000000000 +0200
@@ -16,7 +15,8 @@
 	umstream.c \
 	urlmon_main.c
 
-RC_SRCS = rsrc.rc
+RC_SRCS = rsrc.rc \
+	version.rc

A. Julliard: You shouldn't add multiple resource files, mingw unfortunately doesn't support that. Simply #include version.rc from the main file.

Delay-load

A programmer [July 05] added a wrapper to delay-load uxtheme.dll, as needed by future theming changes.

R. Shearman: You don't need any code for this. Just add uxtheme to DELAYIMPORTS in the Makefile.in and it will be done automagically for you.Wine Archive

Winecfg

A programmer suggested an: option through WineCFG as the WineD3D GLSL code will be experimental for some time.

A. Julliard [May 06]: I don't think that's a reason for exposing such details to the user. If it's not working well enough to be enabled by default, then people who know what they are doing can tweak the code, or possibly set a registry key. wine archive

wine.inf

A programmer asked [Mar 06]L > Would it be acceptable to put some sane defaults in wine.inf and have GetSystemInfo check and fill the correct keys in the registry? Or do we need a way to fill these keys at every boot of the system (start of wineserver)?

A. Julliard: We can either put defaults in wine.inf, or if that's not good enough compute the proper values at run-time. It doesn't really make sense to do both.

M. McCormack added some a fake dll:

Mike McCormack [email redacted]

Wed Mar 22 23:46:31 2006 +0900

wine.inf: Add a shdocvw.dll stub dll to the system directory.

A developer asked: What about adding a few more, like opengl32?

H. Leidekker: Yes, if there's evidence that an app references a dll (or exe) as a file then it should be put into the FakeDllsSection.

Another developer mentioned ddraw.dll

M. McCormack: [send] a patch to wine-patches adding it to wine.inf. wine archive

Spec. Files

A. Hentschel Sept 2010 A specfile declares what functions of a dll are exported(so that an app can use it, or another dll). a stub in a specfile is a function not implemented in wine. if the function to be exported is implemented(mostly) or stubbed in code the it gets exported with a function type e.g. stdcall.

Winehq Bugzilla entry 18347 what does "@ stub" do? Is there any point in adding that, as opposed to a dummy implementation of the function?

A. English: Marks it as unimplemented.[may 09]

Ge van Geldorp [Jun 06]: 32-bit doesn't export Get/SetWindowLongPtrA/W and GetSetClassLongPtrA/W (they are just #defines in winuser.h), while 64-bit has to export them. Is there a portable solution to set a Make variable only when it doesn't have a value yet?

M. McCormack: Is there any problem with just exporting them in win32 also? Last time I talked to Alexandre about this, I think that was the conclusion that was reached. wine archive

 diff -u -p -u -p -r1.19 crypt32.spec
> --- dlls/crypt32/crypt32.spec 10 Nov 2004 01:31:50 -0000 1.19
> +++ dlls/crypt32/crypt32.spec 6 Apr 2005 05:52:55 -0000
> @@ -133,6 +133,7 @@
>  @ stub CryptMsgUpdate
>  @ stub CryptMsgVerifyCountersignatureEncoded
>  @ stdcall CryptProtectData(ptr wstr ptr ptr ptr long ptr)
> +@ stdcall CryptUnprotectData(ptr ptr ptr ptr ptr long ptr)
>  @ stdcall CryptRegisterDefaultOIDFunction(long str long wstr)
>  @ stdcall CryptRegisterOIDFunction(long str str wstr str)
>  @ stub CryptRegisterOIDInfo
> @@ -149,7 +150,6 @@
>  @ stub CryptSignHashU
>  @ stub CryptSignMessage
>  @ stub CryptSignMessageWithKey
> -@ stub CryptUnprotectData
>  @ stub CryptUnregisterDefaultOIDFunction
>  @ stub CryptUnregisterOIDFunction
>  @ stub CryptUnregisterOIDInfo

D. Timoshkov: It's better to keep alphabetical order of .spec file entries.

arch

A wine dev: I [thought] the arch flag is only used to say "this function is only available on win32" or to say the same for win64. We do that e.g. in msvcp* dlls.

He noticed stubs such as @ stub -arch=win32 function with an arch flag caused winebuild to put some extra assembler code into the dll. A. Julliard said this was not intended and merged a patch. http://permalink.gmane.org/gmane.comp.emulators.wine.devel/80755 with 1.3.3. A. Hentschel: the extra assembler stuff didnt hurt something, it was just not needed. Further you can also say things like "-arch arm" to only export a function on arm

Big Endian vs Little

+#ifdef WORDS_BIGENDIAN
+
+/* FIXME: more-optimal versions may be in <byteswap.h>, <sys/bswap.h>, or
+ * <sys/endian.h>.  Use config checks to find 'em.

A. Julliard: No, you don't want config checks, what you want to use here is RtlUshortByteSwap/RtlUlongByteSwap Wine Archive Link

---

> +static cab_ULONG fci_set_little_endian_ulong( cab_ULONG i ) {
> +  unsigned char v[4];
> +  v[0]=i;
> +  v[1]=i>>8;
> +  v[2]=i>>16;
> +  v[3]=i>>24;
> +  return *((cab_ULONG*)(v));
> +}
> +
> +static cab_ULONG fci_get_little_endian_ulong( cab_ULONG i ) {
> +  unsigned char v[4];
> +  cab_ULONG r=0;
> +  memcpy(v,&i,4);
> +  r|=v[3];
> +  r<<=8;
> +  r|=v[2];
> +  r<<=8;
> +  r|=v[1];
> +  r<<=8;
> +  r|=v[0];
> +  return r;
> +}

A. Julliard: You should use the Rtl*ByteSwap functions with the appropriate #ifdef WORDS_BIGENDIAN, it would be a lot more efficient, particularly on little endian platforms that represent 99.9% of the real world cases. Wine Archive

Headers

C. Costa: I plan to flatten the ddraw directory and perform some files renaming in it.

A. Julliard: While you are at it, it would be nice to merge the header files. We really don't need one header for each source file. Wine Archive Link

[Oct 06]

Move cfgmgr32.h to include/ to match the SDK.

A. Julliard: When moving or adding headers, you need to fix include/Makefile.in too. wine archive

Matching PSDK

This doesn't seem to match the PSDK, I don't see anything like this in ws2tcpip.h. J. Latimer [apr 09]: What is the policy for selecting the Platform SDK? �Do we always use the latest (in this case Vista) PSDK? �There does not appear to any guidance on this matter on the various Wine sites.

R. Shearman: It's rare for things to change much from one PSDK version to the next, so there have not been many cases like this before. However, generally newer is better.

Exported Headers

+/******************************************************************************
+ *  RtlCopyAcl      [NTDLL.@]
+ */
+BOOLEAN WINAPI RtlCopyAcl(DWORD nDestinationAclLength, PACL pDestinationAcl, PACL pSourceAcl)

A. Juliard [Jun 05]: I don't see any RtlCopyAcl in ntdll. Where does this come from?

The programmer replied: There isn't an RtlCopyAcl in ntdll, but it simplifies the code a bit and can be useful in other places. For one I should've taken out the [NTDLL.@] because we don't export it, but should the function be named something else?

A. Julliard: Yes, it should be named something like copy_acl to make it clear it's an internal function, and it should definitely not be declared in an exported header. Wine Archives

+/*********************************************************************
+ *      ___lc_codepage_func (MSVCRT.@) + */
+int ___lc_codepage_func(void)
+{
+    return MSVCRT___lc_codepage;
+}
+
+/*********************************************************************
+ *      ___lc_collate_cp_func (MSVCRT.@) + */
+int ___lc_collate_cp_func(void)
+{
+    return MSVCRT___lc_collate_cp;
+}

D. Timoshkov [ apr 09] All msvcrt public exports should have explicit CDECL calling convention.

Undocumented API

A programmer queried: What is our policy on adding undocumented functions to Wine's public headers? [...] There are several places where -Wmissing-declarations warns about missing declarations for exported APIs. A lot of these have no corresponding entry in Microsoft's public headers. Adding these declarations gets rid of the warnings and makes our headers more complete. What are your thoughts on this matter?

R. Shearman: I think we should do this, but protect them with #ifdef __WINESRC__, since we may want to use them internally, but we don't want them used in things that may be built with the PSDK headers, like tests.

A. Julliard: We can do that for things that actually need to be used from other dlls, but if it's just to quiet the warnings then I don't think that's a good enough reason to diverge from the SDK headers.

F. Gouget: Developers who write Windows programs and call undocumented functions are likely to define a prototype for them straight in their source code. So adding prototypes for undocumented functions in Wine's public headers would likely cause such code to fail to compile with Winelib (duplicate prototypes, small divergences, etc). As such it should be avoided. wine archive

#ifdef

Keep in mind that when an #ifdef is really necessary, other may need to be aware of this and make changes too. R. Lunnon: Could I suggest where someone adds #ifdefs for linux, bsd, etc such as in the recent GlobalMemoryStatusEx issue I just fixed, that the original authors include an #else clause which raises a warning to alert us poor Solaris (and other OS) maintainers to the fact we have to do something. In the meantime I might have to search for OS specific things not flagged with warnings or fixmes..wine archive

A programmer posted a patch for cdrom.c using #ifdef

D. Timoshkov: Please do not introduce even more platform dependent #ifdefs into cdrom.c, currently it already has enough mess with all that #if defined(linux)/(__FreeBSD__)/(__NetBSD__). Add proper configure checks for headers and structures your platform needs. Wine Archive

+#if defined(linux)
+    /* Check we have a SCSI device and a supported driver */

D. Timoshkov: Please use appropriate configure checks instead of a blind '#if defined(linux)'. Wine Archive

The programmer then resubmitted for review: Check for HAVE_SG_IO_HDR_T_INTERFACE_ID instead of if defined (Linux)

+static NTSTATUS GetInquiryData(int fd, PSCSI_ADAPTER_BUS_INFO BufferOut, DWORD OutBufferSize)
+{
+    PSCSI_INQUIRY_DATA inquirydata = NULL;
+    sg_io_hdr_t iocmd;
     ^^^^^^^^^^^^^^^^^
+    UCHAR inquiry[INQ_CMD_LEN] = {INQUIRY, 0, 0, 0, INQ_REPLY_LEN, 0};
+    UCHAR sense_buffer[32];
+    struct hd_driveid;
+    int iochk, version;
+
+#ifdef HAVE_SG_IO_HDR_T_INTERFACE_ID
+    /* Check we have a SCSI device and a supported driver */
+    if(ioctl(fd, SG_GET_VERSION_NUM, &version) != 0)

D. Timoshkov: This won't compile if sg_io_hdr_t isn't present. The Underlined structure declaration needs to be protected as well.

Just add '#undef HAVE_SG_IO_HDR_T_INTERFACE_ID' after config.h include and try to compile.

#include

a/include/wine/debug.h      2006-05-16 20:37:38.000000000 +0100 +++
b/include/wine/debug.h      2006-05-16 21:08:26.000000000 +0100 @@
-148,6 +148,8 @@   const char *function, const char *format, va_list args ); };

+extern void debug_init(void);
+extern unsigned char __wine_dbg_get_channel_flags( struct__wine_debug_channel *channel );
extern int __wine_dbg_set_channel_flags( struct __wine_debug_channel *channel,

A. Juliard: [May 06]: debug_init() is an internal function and there's no reason to add it to the header file.

E. Pouch: [patches for] missing prototypes for some functions [...] is trickier, especially for internal/hidden prototypes, for which we don't want to publish them (debug_init() being one of them) wine archive

wctype.h

> --- dlls/riched20/editstr.h	2 Mar 2006 11:18:19 -0000	1.27
> +++ dlls/riched20/editstr.h	5 Mar 2006 06:36:50 -0000
> [...] #include <stdlib.h>
> +#include <wctype.h>

A. Julliard: You can't use anything from wctype.h in Wine. Use the functions from wine/unicode.h instead. wine archive

Config.h

 +#include "config.h"
 +#include <stdarg.h>
 +#include <stdio.h>
 +#include <string.h>

D. Timoshkov: if you are not going to conditionally include headers using '#ifdef HAVE_xxx' there is no need to include config.h.

A Programmer posted: I need to be able to include itss.h from dlls/itss in order to read CHM files. [...] I added -I($SRCDIR)../itss to hhctrl's Makefile.in. The next problem comes when I run make depend. make depend looks for itss.h but can't find it because it hasn't been built yet. [it was] then suggested adding ../dlls/itss/itss.idl to IDL_SRCS in include/Makefile.in. That works fine up until make install which gives these two errors:

/usr/bin/install: cannot create regular file `/usr/include/wine/windows/../dlls/itss/itss.idl': No such file or directory
/usr/bin/install: cannot create regular file `/usr/include/wine/windows/../dlls/itss/itss.h': No such file or directory

Do you have any suggestions on how to access itss.h from hhctrl.ocx?

A. Julliard: No, you can't do that sort of thing. You have to move the idl file to the include directory (probably in include/wine). Wine Archive

---

 +#ifdef HAVE_LINUX_VIDEODEV_H
 +#include <stdio.h>
 +#include <sys/ioctl.h>
 +#include <sys/stat.h>
 +#include <errno.h>
 +#include <asm/types.h>
 +#include <linux/videodev.h>
 +#include <fcntl.h>
 +#include <unistd.h>
 +#include "winnls.h"
 +#endif

J. Lang:You need to include "winnls.h" unconditionally, capGetDriverDescriptionW is always compiled with calls to MultiByteToWideChar. Wine Archive Link

---

on Darwin/Mac OS X, there are still compilation errors related to winsock.h inclusion. The error was:
../../../include/winsock.h:414: error: redefinition of 'struct timeval'
My solution is to include "wine/test.h" after every other header.

A. Julliard: You should fix wine/test.h and/or winsock.h to work properly in all cases. Otherwise the problem will keep coming back.

Dependencies Matching the Windows Platform SDK

+#include <stdarg.h>
+
+#include "windef.h"
+#include "winbase.h"
+#include "winnt.h"
+#include "winreg.h"
+#include "winternl.h"

M. McCormack: 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.

F.Gouget: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.

S. Edwards[Aug 05]: I work to keep the Wine SDK compatible with the PSDK because we have developed a MSVC backend in the ReactOS build system and one day (God willing) I will get the ReactOS Project to use Wine headers rather than w32api headers. As such it needs to be able to compile applications on MSVC and gcc. Wine Archive

A programmer wondered: SDK is available on msdn (web site I mean) with most of the information we need. Can we write our own DDK headers just as we can write the SDK ones?

Ivan Leo Pouti: MSDN has the documentation, and only the documentation, subject to these TOS

• http://msdn.microsoft.com/library/default.asp?url=/library/en-us/sdkintro/sdkintro/legal_information_sdk.asp
• http://msdn.microsoft.com/library/default.asp?url=/library/en-us/kmarch/hh/kmarch/DDKLegal.asp

As you'll note the info for the first half page is actually the same. The SDK headers, like the DDK ones, are *not* on MSDN, you have to download or order the SDK and accept its EULA to access the headers.

Wine Archive [Ed. There was several views expressed in the forums about this topic. Laws are different, depending upon the country and are subject to change. A wiki is a community effort where comments are added by those unqualified in legal matters. Readers need to obtain qualified professional advice. Wine-Wiki.org:General disclaimer ]

#Define

> +#define WC_STATICA	"Static"
> +#if defined(__GNUC__)
> +# define WC_STATICW (const WCHAR []){ 'S','t','a', 't','i','c',0 }
> +#elif defined(_MSC_VER)
> +# define WC_STATICW L"Static"
> +#else
> +static const WCHAR WC_STATICW[] = { 'S','t','a','t','i','c',0 };
> +#endif
> +#define WC_STATIC		WINELIB_NAME_AW(WC_STATIC)

A programmer: I'm curious, what is the reason for the special case handling for MSVC and GCC when we have to do the portable way anyway? Why do we _additionally_ provide two unportable ways when there is a portable one?

D. Timoshkov: (const WCHAR []) cast is not portable (gcc only) and L"string" syntax produces different results on different platforms depending on the compiler's builtin wchar_t size.

M. McCormack: I can see two reasons:

1. The portable way adds a (non-standard) symbol.
2. The static string declaration may not be discarded by some compilers, so it may needlessly increase the size of our binaries.

Anon: (1) [the portable way] could be fixed by adding a _ prefix.

M. McCormack: Adding another prefix fixes nothing. The original Windows SDK headers don't export a symbol, and we do. It's simply good practice to try and be as close as possible to the original SDK.

Anon: [For (2)] Unless MSVC or GCC do not do this optimization (and I'm sure both do) this is no argument. Actually those 2 non-standard ways put additional burden on the optimizer - if one of those wide string literals is used multiple times the optimizer has to make sure that the compiler doesn't generate a copy for each reference (and I'm not so sure if GCC / MSVC do this, especially since writeable strings (yea, it's const but still) were only recently forbidden by GCC and I guess they are allowed in MSVC). [the discussion ended at this point] Wine Archive

L Defines

A developer commented: I'm currently dealing with the DllRegister part of wintrust. One of it's include files (softpub.h) on Windows has:

#define SP_POLICY_PROVIDER_DLL_NAME L"WINTRUST.DLL" 
#define SP_INIT_FUNCTION            L"SoftpubInitialize"
#define SP_OBJTRUST_FUNCTION        L"SoftpubLoadMessage" 
#define SP_SIGTRUST_FUNCTION        L"SoftpubLoadSignature" [...]

should we do the same?

M. Meissner: They won't work when used in WINE. L"xxx" usually gives a strings with 4 byte characters instead of the expect 2.

F. Richter: You could look how e.g. the WC_* macros are defined in commctrl.h. wine archive

Definitions which conflict with unix

+#ifdef USE_WS_PREFIX
+#define WS(x)    WS_##x
+#else
+#define WS(x)    x
+#endif
+
+struct WS(in6_addr) {
+    union {
+        WS(u_char)  Byte[16];
+        WS(u_short) Word[8];
+    } u;
+} IN6_ADDR, *PIN6_ADDR, *LPIN6_ADDR; +
+#define in_addr6 WS(in6_addr)
+
+#define _S6_un     u
+#define _S6_u8     Byte
+#ifndef USE_WS_PREFIX
+#define s6_addr    _S6_un._S6_u8
+#else
+#define WS_s6_addr _S6_un._S6_u8
+#endif
+
+#define s6_bytes   u.Byte
+#define s6_words   u.Word

A. Juliard: apr 09You need the WS prefix on all definitions that can potentially conflict with Unix.

T. Trummer added some definitions for irda and queried [sept 09]: The WS_ prefixes only get added to symbols which collide with the corresponding linux header files? So basically I create a small test file with

1. include <sys/socket.h>
2. include <linux/irda.h>
3. include "af_irda.h"

and check for collisions. J. Lang wrote: The WS_ prefixes only get added to symbols which collide with the corresponding linux header files

Define Magic Numbers

+ todo_wine { ok(hr==0x800401e4, "MkParseDisplayName - Progid not implemented hr=%08x\n", (int) hr); }

M. McCormack: 0x800401e4 is defined as MK_E_SYNTAX. It would be clearer to use the define rather than a number.

+#define UNKNOWN_RES_ERR1 0x8007007E 

J. Lang: This is HRESULT_FROM_WIN32(ERROR_MOD_NOT_FOUND). Any 0x8007* error code is an HRESULT_FROM_WIN32 of a Win32 error. Wine Archive

---

Define Naming Styles

+#define UNICODE_CP 1200

D. Timoshkov: 'IMO it's better to name it CP_UNICODE. It sounds more natural to define it that way since we already have CP_ACP, CP_OEMCP, CP_MACCP. This is as dlls/mlang/mlang.c has, and it's a more consistent value with include/ddeml.h':

#define CP_WINUNICODE 1200

and PSDK's mimeole.idl:

const CODEPAGEID CP_UNICODE = 1200;

---

 +#ifndef _POWRPROF_H
 +#define _POWRPROF_H 1

Most of the Wine headers use the prefix __WINE_ [This would become] __WINE_POWRPROF_H

---

>  #define cffileCONTINUED_PREV_AND_NEXT  (0xFFFF)
> +#define ifoldCONTINUED_PREV_AND_NEXT   (0xFFFF) /* original name defined by MS */

A. Julliard [Jun 05]: If these are the names defined by MS you should get rid of the others, there's no sense in having two different names for the same thing.Wine Archive

---

WIN32_LEAN_AND_MEAN

+#define WIN32_LEAN_AND_MEAN
+
+#include "config.h"
+#include "wine/port.h"
+#include "wine/library.h"
+#include "wine/debug.h"

D. Timoshkov [Oct 06]: WIN32_LEAN_AND_MEAN is used only by windows.h to omit some optional includes. Since inclusion of windows.h is forbidden in Wine DLLs defining WIN32_LEAN_AND_MEAN is not needed. wine archive

Macros

M. McCormack [Oct 06]: All upper case is usually for macros... wine archive

Dimi Paun [July 05]: (one of) the problem with macros [is that the resulting code doesn't always look like C In fact, it can look rather magical]. It's best if their usage is not very different then a regular C construct. Principle of least surprise and all that. Wine Archive

-            if ((hr = callback(hinf, buffer, arg)) != S_OK) 
+ if (FAILED(hr = callback(hinf, buffer, arg)))

M.McCormack [May 06]: It's usually a bad idea to do function calls or assignments inside macros... in this case it's safe, but sometimes the macro is something like:

1. define FAILED(hr) (((hr)>0xf00) && (hr)<0xb00))

then the function would get called twice... It's even more fun if you do:

FAILED(hr++)

wine archive

> +    p_fci_internal->perf->erfOper = FCIERR_NONE;
> +    p_fci_internal->perf->erfType = ERROR_INVALID_DATA;
> +    p_fci_internal->perf->fError = TRUE;
> +    SetLastError(ERROR_INVALID_DATA);

M. McCormack [July 05]: Your code seem to be doing this over and over again. Is there any reason not to make a macro so you can do something like:

FCI_SET_ERROR(FCIERR_NONE, ERROR_INVALID_DATA );

maybe something like this Wine Archive:

#define FCI_SET_ERROR( oper, type ) \
p_fci_internal->perf->erfOper = oper; \
p_fci_internal->perf->erfType = type; \
p_fci_internal->perf->fError = TRUE; \
SetLastError(type);

When to use a Function

+#define expect_buf_offset(_hmmio, _off) \ 
+do \
+{ \
+    MMIOINFO _mmio; \
+    LONG _ret; \
+    memset(&_mmio, 0, sizeof(_mmio)); \ 
+    _ret = mmioGetInfo((_hmmio), &_mmio, 0); \ 
+    ok(_ret == MMSYSERR_NOERROR, "mmioGetInfo error %u\n", _ret); \ 
+    ok(_mmio.lBufOffset == 0, "expected 0, got %d\n", _mmio.lBufOffset); \ 
+    _ret = mmioSeek((_hmmio), 0, SEEK_CUR); \ 
+    ok(_ret == (_off), "expected %d,> got %d\n", (_off), _ret); \ 
+} \
+while (0)

A. Juliard: [jan 10] Please avoid such multi-line macros, define a function instead. http://permalink.gmane.org/gmane.comp.emulators.wine.devel/75345

INVALID_FILE_SIZE

A. Mohr [Oct 05]: you're not using the INVALID_FILE_SIZE macro that I really, really think should be used since it's been created [...] for this purpose (and for the even better purpose of never managing to write/edit/delete a 0xFFFFFF or 0xFFFFFFF instead...)

The programmer queried whether it was necessary in this case: I agree that the INVALID_FILE_SIZE should be used. However, INVALID_FILE_SIZE macro is exactly 0xFFFFFFFF. Furthermore, this code does work with file sizes of 0x12345678FFFFFFFF. Have a look at the MSDN documentation. Alternatively, GetFileSizeEx could be used.

A. Mohr felt it was still good practice: using INVALID_FILE_SIZE ensures that you may never potentially mis-write or mis-modify 0xffffffff. wine archive

ERROR_SUCCESS

+    SetLastError(0);

One developer said: 0 is ERROR_SUCCESS [...] wine archive A. Julliard: SetLastError(0) is fine.

The Path for .wine
A Programmer asked: What's the best way/way usually used in wine to get the full path to .wine?

wine_get_config_dir.

PathAddBackslash

> +#define ADD_SLASH(x) do { char *t; if (x[0]) {t=x+strlen(x)-1;if(t[0] > != '\\' ) {t[1] ='\\';t[2] ='\0';}}} while (0) +

J. Hawkins [Sept 06]: This is a really bad macro. Please use PathAddBackslash.

Declaring Variables

 +    DWORD DEADFFFF = 0xDEADFFFF;  /* Constants */
 +    DWORD ZERO     = 0;
 +    WORD  THREE    = 0x3;        

M. McCormack: In C, variables are traditionally lower case, and macros are upper case.

Declaring and Renaming Variables

Declaring

case WM_CAP_SET_CALLBACK_FRAME:
{
^^
    UINT something;

R. Kalbermatter: You can only declare variables in standard C at the beginning of a block and since Wine has to be compilable in standard C you need to add curly braces before you can declare variables.

 int PASCAL WinMain (HINSTANCE hInstance, HINSTANCE prev, LPSTR cmdline, int show)
 {
+    if(getenv("WINE_NO_MENU_ENTRIES"))
+    {
+        return 0;
+    }
+
     LPSTR token = NULL, p;

D. Riekenberg [feb 09]: Declaring a variable after code is C99 and not allowed in Wine.

+    uint i;

A. Julliard May 09 :uint is not a standard type. Use "unsigned int" instead.

Meaningful names

+    WCHAR buf[21];
+    WCHAR buf2[21];

M. Hearn: It would be nicer to call the variables something more meaningful than buf1, buf2.Wine Archive Link

Renaming Variables

 static void write_function_stubs(type_t *iface)
  {
 -    func_t *cur = iface->funcs;
 -    char *handle_name = get_attrp(iface->attrs, ATTR_IMPLICIT_HANDLE);
 +    char *implicit_handle = get_attrp(iface->attrs, ATTR_IMPLICIT_HANDLE);
 +    int explitit_handle = is_attr(iface->attrs, ATTR_IMPLICIT_HANDLE);
 +    func_t *func = iface->funcs;
 +    var_t* var;

A. Julliard: Your patch would be a lot easier to review if you avoided renaming existing local variables. If you really think the names need to be changed please do it in a separate patch. Wine Archive Link

.tlb files

A developer wrote [jan 08]: while debugging problems with and old version of Sonic Stage, I found that sonic stage includes broken type libraries, i.e. the TYPEFLAG_FDISPATCHABLE bit is generally missing. As widl (and probably midl too) set this bit automatically, I have some problems in creating a .tlb file for a test case for my one-line patch that also misses this bit. There are some possibilities to get around the problem:

a) Include a broken (hand-patched) tlb file as binary file in git

b) Include a program that breaks tlb files and call it while building tests

c) Include tlb file patching into the testcase (i.e. copy a good tlb, patch it, and perform some tests with the copy, than delete it)

Which approach would have the highest chance of getting the fix (including the testcase that tests broken type libraries) included into Wine?

A. Julliard: c) is the way to go.

Windows Compatability

A developer queried [Feb 07]: I find some function

LONG WINAPI AVIStreamSampleToTime(PAVISTREAM pstream, LONG lSample)

(in the avifil32 dll), the second parameter should really be an ULONG instead of a LONG. It'd fix the signedness problems. My question is, is such a change allowed?

M. Stefanuic: If it is a official WinAPI you have to conform to what Windows has there. If it is an internal function you can change it.

F. Gouget:Our headers must match the PSDK ones otherwise it would cause trouble for people trying to recompile Windows code with them. Since the PSDK says the second parameter is a LONG, that's what we have to use too.

Wine32 strings

winedeve feb 10 I'm unsure whether it's actually possible to find %d/%ld formats that work on both systems for all types one comes across (UINT, MMRESULT etc.) What's the receipe?

A. Julliard: All Win32 types are the same across platforms and can be printed without issues (except on Mingw but we don't care). Standard C types like size_t vary across platforms and should be avoided in traces. M. Stefanuic: WTB uses Mingw to build the tests and thus those warnings should be ignored.

V. Margolen: You should always use SendMessageW where possible. Especially for things that don't care if it's A or W. This is to prevent extra A->W conversions.http://permalink.gmane.org/gmane.comp.emulators.wine.devel/69662

+    SNDMSGW(hwndLV, LVM_SETITEMSTATE, (WPARAM)(INT)(iIndex), (LPARAM) (LPLVITEMA)&_LVi);}

V. Margolen may 09: If you using SendMessageW you must use W structures (LVITEMW not LVITEMA).Also why are you typecasting WP & LP two times?

Long

[Sept 09] A. J wrote: You can't simply use long here, you need to handle the difference in the size of long between Win32 and Unix. E. Pouch queried this as he pointed out the submitted patch was following the existing examples. A. J wrote: Sure, but that needs to be fixed. Since you are reimplementing it you should do it right.

Win64 compatability

K. Koltzau [July 05]: gcc and msvc++ have different opinions on the size of a long in 64bit code, gcc has sizeof(long)==8 while msvc++ has sizeof(long)==4

Binary compatibility with win64 will be just about impossible as the long datatype is used extensively throughout the headers and code. Doing -Dlong=int works in simple cases, but not as a general rule

J. Lang: What about defining LONG as int in win64?

K. Koltzau: you could start with running "grep long *" inside the include directory. Every instance would need to be changed in some form, I count well over a thousand instances in just the headers.

D. Timoshkov: We need to replace 'long' by 'LONG' and 'int' by 'INT' in the headers. That's a job for a simple script. Then synchronize headers with an actual implementation, that's a little bit harder since we need to closely inspect every API implementation.

Raphael asked : what about .spec files and winebuild ? [The discussion apparently ended there] Wine Archive