WineHQ

Wine Developer's Guide/Coding Practice: Difference between revisions

(→‎Writing portable code: added Alexandre's recent explanation)
 
(12 intermediate revisions by 3 users not shown)
Line 2: Line 2:
This chapter describes the relevant coding practices in Wine, that you should be aware of before doing any serious development in Wine.
This chapter describes the relevant coding practices in Wine, that you should be aware of before doing any serious development in Wine.


== Patch Format ==
== Some notes about style ==


Patches are submitted via email to the Wine developers' mailing list, [mailto:[email protected] [email protected]] (subscribe to the list before emailing a patch). Your patch should include:
In the time since the Wine project was started in 1993, its developers have settled on the following general style conventions. These conventions are merely matters of taste; although some of them were once required for compiler compatibility, the current standards for compiler compatibility are documented in the [[#Writing portable code|Writing portable code]] section below.


* A meaningful subject (very short description of patch), beginning with [PATCH]
Some maintained subsystems use different styles, some of which conflict with the below. In this case you should try to conform to the existing style. At the same time, some old or unmaintained code uses style that conflicts with the below, and in that case the style is seen as ugly and we'd prefer to change it. On the other hand, patches which do nothing but change style are unnecessary noise, not worthwhile, and should be avoided.
* A long (paragraph) description of what was wrong and what is now better (recommended).
* A change log entry (short description of what was changed).
* The patch in “Git” format


To generate a patch using Git, first commit it to your local tree.
The following styles are encouraged in general:


If you are adding a file, use <b>git add</b>. Commit the change using <b>git commit</b>. The commit message will be sent with your patch, and recorded in the commit log.
* Above all, please use a consistent style in your own changes
* Use 4-space indentation instead of tabs
* Avoid very long lines ([https://www.winehq.org/pipermail/wine-devel/2012-September/097024.html there is no hard limit], the preferred length is 100, but 120 or 80 chars are fine too)
* Use only slash-star comments, never slash-slash comments (e.g. write <code>/* */</code> instead of<code>//</code>)
* Use snake_case for internal variables instead of camelCase or TitleCase
* Prefer asterisks to declare pointers instead of the P and LP typedefs (e.g. write <code>DWORD *</code> instead of <code>LPDWORD</code>)
* Asterisks should stick to the variable, not the type (e.g. write <code>int *foo</code> instead of <code>int* foo</code>)
* Limit type casts as much as possible (e.g. don't cast a <code>void *</code> to a <code>DWORD *</code> or vice-versa)
* Don't use Hungarian notation (e.g. write <code>char *important_buffer</code> instead of <code>char *pImportantBuffer</code>)
* Always put braces on a separate line. Braces should not be indented relative to the previous line. This applies to all braces: control flow, functions, type definitions, and initializers.
** Exception: an array or struct initializer (including as part of another initializer) can be specified on one line if it's short enough.
** Exception: in a do-while statement, the "while" statement should be placed on the same line as the closing brace, so that it's clear it's not a standalone "while".
* Write hexadecimal constants and variable suffixes in all lowercase (e.g. <code>0xcafe1234u</code>)
* Put spaces around arithmetic and boolean operators (e.g. write <code>(1 + 2) * 3</code> instead of <code>(1+2)*3</code>)
* Use the <code>ARRAY_SIZE</code> macro where appropriate
* Remove trailing whitespace from your changes, but don't change whitespace in other lines
* Comment out code with <code>if (0)</code>, to ensure that the code compiles:
<source lang="c">
/* note about reason for commenting block */
if (0)
{
    code
    code /* comments */
    code
}
</source>
* Use <code>void</code> when declaring or defining parameter-less functions:
<source lang="c">
int foo();                      /* Incorrect */
int foo() { }                  /* Also incorrect */


After committing the patch, you can extract it using <b>git format-patch</b> and send it to wine-devel using <b>git send-email</b>.
int foo(void);                  /* Much better */
int foo(void) { }              /* Also much better */
</source>
* Declarations cannot follow statements; this will trigger a compiler warning. Declarations in a for loop initializer are fine, though.


== Some notes about style ==
=== Preferred D3D coding style ===


There are a few conventions about coding style that have been adopted over the years of development. The rational for these “rules” is explained for each one.
Direct3D and DirectShow enforce a more restrictive coding style than most of the rest of Wine. The following describes Direct3D coding style. Note that many of these principles are applicable elsewhere.


<ul>
* Describe exactly what you're changing in the patch subject line instead of, e.g., a generic 'fix something'.
<li><p>No HTML mail, since patches should be in-lined and HTML turns the patch into garbage. Also it is considered bad etiquette as it uglifies the message, and is not viewable by many of the subscribers.</p></li>
* Trace messages on function entry should include variable names. This makes debugging much easier, as you then don't have to look up the parameter order.
<li><p>Only one change set per patch. Patches should address only one bug/problem at a time. If a lot of changes need to be made then it is preferred to break it into a series of patches. This makes it easier to find regressions.</p></li>
* Use <code>%#x</code> when tracing hexadecimal numbers (e.g. do not use <code>%x</code>, <code>0x%x</code>, <code>%08x</code>, etc.). We want the "0x" prefix so it's always clear the number is hexadecimal, and %#x is one less character than 0x#x.
<li><p>Tabs are not forbidden but discouraged. A tab is defined as 8 characters and the usual amount of indentation is 4 characters.</p></li>
* Trace floats using <code>%.8e</code>, and doubles using <code>%.16e</code>. This always gives enough precision to determine the unique number.
<li><p>C++ style comments are forbidden since some compilers choke on them.</p></li>
* Always end trace messages with a period.
<li><p>Commenting out a block of code is usually done by using <code>if (0)</code>. For example:</p>
* Always use 8 space indentation when continuing a previous line (instead of e.g. trying to align to the first function parameter). This avoids wasting space when a continuation must begin late in the line. For example:
<source lang="c">/* note about reason for commenting block */
<source lang="c">
if (0) {
    TRACE("output %p, format %s, scanline_ordering %#x.\n",
code
            output, debug_d3dformat(format_id), scanline_ordering);
code /* comments */
</source>
code
* When continuing a previous line, <code>||</code> and <code>&&</code> should begin the line, rather than ending the previous line. This is a minor visual aid that the line is a continuation of a condition and not a standalone statement.
}</source>
* Don't introduce typedefs, except for function types. Always using e.g. <code>struct</code> and <code>*</code> makes it clearer what kind of object you're dealing with.
<p>The reason for using this method is that it does not require that you edit comments that may be inside the block of code and it makes sure the code gets maintained.</p></li>
* Put the body of an if statement on the next line, even if it's short. It can be easy to miss otherwise when reading.
<li><p>Patches should be in-lined (if you can configure your email client to not wrap lines), or attached as plain text attachments so they can be read inline. This may mean some more work for you. However it allows others to review your patch easily and decreases the chances of it being overlooked or forgotten.</p></li>
* Braces need not be used when an "if" body is only a single line, but if an "if" body has braces, the "else" body should too, and vice versa.
<li><p>Code is usually limited to 100 columns. It generally makes code easier to read.</p></li>
* "case" labels should be indented relative to the "switch", and the body of a case label should be indented relative to the label.
<li><p>If the patch fixes a bug in Bugzilla please provide a link to the bug in the comments of the patch. Also, mark the bug as “resolved fixed” after your patch gets committed.</p></li></ul>
* An empty line between switch cases is preferred. An exception is if (almost) all of the cases have one-line bodies (i.e. "return X"); in this case it is also acceptable to put the body on the same line as the label.
* Use <code>sizeof(*var)</code> in allocations instead of <code>sizeof(type)</code>.
* Sort variable declarations to a reverse Christmas tree by length.
* Always use unsigned types where the type doesn't need to be signed.


=== Inline attachments with Outlook Express ===
== Writing portable code ==


Outlook Express is notorious for mangling attachments. Giving the patch a <tt>.txt</tt> extension and attaching will solve the problem for most mailers including Outlook. Also, there is a way to enable Outlook Express to send <tt>.diff</tt> attachments.
Write code that compiles on recent versions of both GCC and Clang, as well as MSVC if the code is in a PE module. That means:


You need the following two things to make it work.
* Avoid compiler extensions:
<source lang="c">
typeof(D3DCreateBlob) pD3DCreateBlob;                  /* GCC extension */
HRESULT (WINAPI *pD3DCreateBlob)(SIZE_T, ID3DBlob **);  /* Works on both GCC and MSVC */
 
ch += offset ?: size            /* GCC extension */
ch += offset ? offset : size    /* Works on both GCC and MSVC */
</source>
* When declaring function pointers, put the calling convention inside the parentheses:
<source lang="c">
HRESULT WINAPI (*pD3DCreateBlob)(SIZE_T, ID3DBlob **);  /* Works on GCC but not MSVC */
HRESULT (WINAPI *pD3DCreateBlob)(SIZE_T, ID3DBlob **);  /* Works on both GCC and MSVC */
</source>
* Use types with dependable definitions:
<source lang="c">
long wrong = 0xcafe1234l;      /* long is 4 bytes in Win64 but 8 bytes in Unix64 */
LONG right = 0xcafe1234;      /* LONG is 4 bytes, the same as int, on both platforms */
</source>
* Use WCHAR and array syntax for Unicode string literals in non-PE modules ([https://gitlab.com/mywinetools/mywinetools/raw/master/wstr.py helpful tool]):
<source lang="c">
const WCHAR str1[] = L"Hello";  /* Preferred on PE modules. But it won't compile on non-PE modules. */
const WCHAR str2[] = {          /* Tedious, but correct */
    'H','e','l','l','o',0
};
</source>
* Don't use designated initializers for nameless structs or unions ([https://bugs.winehq.org/show_bug.cgi?id=50378 it breaks GCC 4.3])
* Don't declare a typedef more than once ([https://gitlab.winehq.org/wine/wine/-/merge_requests/3059 it breaks GCC 4.3])


# Make sure that <tt>.diff</tt> files have “\r\n” line endings, because if OE detects that there are none it switches to quoted-printable format attachments.
# Using <b>regedit</b> add key <tt>Content Type</tt> with value <tt>text/plain</tt> to the <tt>.diff</tt> extension under <tt>HKEY_CLASSES_ROOT</tt> (same as for <code>.txt</code> extension). This tells OE to use <tt>Content-Type: text/plain</tt> instead of <tt>application/octet-stream</tt>.


Item #1 is important. After you hit the Send button, go to Outbox and using Properties verify the message source to make sure that the mail has the correct format. You might want to send several test emails to yourself too.
There is no strict minimum compiler version. In 2023, Wine's chief maintainer Alexandre Julliard gave the following [https://www.winehq.org/mailman3/hyperkitty/list/wine-devel@winehq.org/message/DKAAZ442SQL7UHQHJZR4ACM4DLI3MCN5/ explanation of Wine's compiler support policy]:


=== Preferred D3D coding style ===
<blockquote style="font-size:small">
Our policy is to not draw arbitrary lines of supported/unsupported. That's true for C standard versions, compiler versions, distros, window managers, graphics cards, etc.


<ul>
We want to support what our users are actually using. If someone reports an issue with their compiler, we want to look into it. We are not going to reject a report because the compiler is "too old", or out of maintenance, or on a distro we've hever heard of.
<li><p> Sort variable declarations to a reverse pyramid by length. </p></li>
<li><p> Use 8 space continuation indent. For example:</p></li>
<source lang="c">
    TRACE("output %p, format %s, scanline_ordering %#x.\n",
            output, debug_d3dformat(format_id), scanline_ordering);
</source>
<li><p> Use British spelling. </p></li>
<li><p> Add a period at the end of trace messages. </p></li>
<li><p> Put the body of an if statement on the next line even if it's short. </p></li>
<li><p> Use sizeof(*var) in heap_alloc() instead of sizeof(type). </p></li>
<li><p> Use ARRAY_SIZE(). </p></li>
</ul>


=== Alexandre's Bottom Line ===
That doesn't mean we have to fix everything, there's always a trade-off between the complexity/ugliness of the fix, the number of affected users, the availability of a workaround, etc. But that should be decided based on the actual issue, not on some arbitrary version number cutoff.


“The basic rules are: no attachments, no MIME crap, no line wrapping, a single patch per mail. Basically if I can't do <code>cat raw_mail | patch -p1</code> it's in the wrong format.”
So it may be that we want to require support for typedef redefinitions, because avoiding them is ugly. In practice, that would mean that you can't build with gcc 4.3 out of the box. But it does not mean that gcc 4.3 is somehow "unsupported". If someone reports some other issue with gcc 4.3, and that one is easy to fix, we'll still want to fix it.
</blockquote>


== Quality Assurance ==
== Quality Assurance ==

Latest revision as of 01:38, 22 November 2023

__NUMBEREDHEADINGS__ This chapter describes the relevant coding practices in Wine, that you should be aware of before doing any serious development in Wine.

Some notes about style

In the time since the Wine project was started in 1993, its developers have settled on the following general style conventions. These conventions are merely matters of taste; although some of them were once required for compiler compatibility, the current standards for compiler compatibility are documented in the Writing portable code section below.

Some maintained subsystems use different styles, some of which conflict with the below. In this case you should try to conform to the existing style. At the same time, some old or unmaintained code uses style that conflicts with the below, and in that case the style is seen as ugly and we'd prefer to change it. On the other hand, patches which do nothing but change style are unnecessary noise, not worthwhile, and should be avoided.

The following styles are encouraged in general:

  • Above all, please use a consistent style in your own changes
  • Use 4-space indentation instead of tabs
  • Avoid very long lines (there is no hard limit, the preferred length is 100, but 120 or 80 chars are fine too)
  • Use only slash-star comments, never slash-slash comments (e.g. write /* */ instead of//)
  • Use snake_case for internal variables instead of camelCase or TitleCase
  • Prefer asterisks to declare pointers instead of the P and LP typedefs (e.g. write DWORD * instead of LPDWORD)
  • Asterisks should stick to the variable, not the type (e.g. write int *foo instead of int* foo)
  • Limit type casts as much as possible (e.g. don't cast a void * to a DWORD * or vice-versa)
  • Don't use Hungarian notation (e.g. write char *important_buffer instead of char *pImportantBuffer)
  • Always put braces on a separate line. Braces should not be indented relative to the previous line. This applies to all braces: control flow, functions, type definitions, and initializers.
    • Exception: an array or struct initializer (including as part of another initializer) can be specified on one line if it's short enough.
    • Exception: in a do-while statement, the "while" statement should be placed on the same line as the closing brace, so that it's clear it's not a standalone "while".
  • Write hexadecimal constants and variable suffixes in all lowercase (e.g. 0xcafe1234u)
  • Put spaces around arithmetic and boolean operators (e.g. write (1 + 2) * 3 instead of (1+2)*3)
  • Use the ARRAY_SIZE macro where appropriate
  • Remove trailing whitespace from your changes, but don't change whitespace in other lines
  • Comment out code with if (0), to ensure that the code compiles:
/* note about reason for commenting block */
if (0)
{
    code
    code /* comments */
    code
}
  • Use void when declaring or defining parameter-less functions:
int foo();                      /* Incorrect */
int foo() { }                   /* Also incorrect */

int foo(void);                  /* Much better */
int foo(void) { }               /* Also much better */
  • Declarations cannot follow statements; this will trigger a compiler warning. Declarations in a for loop initializer are fine, though.

Preferred D3D coding style

Direct3D and DirectShow enforce a more restrictive coding style than most of the rest of Wine. The following describes Direct3D coding style. Note that many of these principles are applicable elsewhere.

  • Describe exactly what you're changing in the patch subject line instead of, e.g., a generic 'fix something'.
  • Trace messages on function entry should include variable names. This makes debugging much easier, as you then don't have to look up the parameter order.
  • Use %#x when tracing hexadecimal numbers (e.g. do not use %x, 0x%x, %08x, etc.). We want the "0x" prefix so it's always clear the number is hexadecimal, and %#x is one less character than 0x#x.
  • Trace floats using %.8e, and doubles using %.16e. This always gives enough precision to determine the unique number.
  • Always end trace messages with a period.
  • Always use 8 space indentation when continuing a previous line (instead of e.g. trying to align to the first function parameter). This avoids wasting space when a continuation must begin late in the line. For example:
    TRACE("output %p, format %s, scanline_ordering %#x.\n",
            output, debug_d3dformat(format_id), scanline_ordering);
  • When continuing a previous line, || and && should begin the line, rather than ending the previous line. This is a minor visual aid that the line is a continuation of a condition and not a standalone statement.
  • Don't introduce typedefs, except for function types. Always using e.g. struct and * makes it clearer what kind of object you're dealing with.
  • Put the body of an if statement on the next line, even if it's short. It can be easy to miss otherwise when reading.
  • Braces need not be used when an "if" body is only a single line, but if an "if" body has braces, the "else" body should too, and vice versa.
  • "case" labels should be indented relative to the "switch", and the body of a case label should be indented relative to the label.
  • An empty line between switch cases is preferred. An exception is if (almost) all of the cases have one-line bodies (i.e. "return X"); in this case it is also acceptable to put the body on the same line as the label.
  • Use sizeof(*var) in allocations instead of sizeof(type).
  • Sort variable declarations to a reverse Christmas tree by length.
  • Always use unsigned types where the type doesn't need to be signed.

Writing portable code

Write code that compiles on recent versions of both GCC and Clang, as well as MSVC if the code is in a PE module. That means:

  • Avoid compiler extensions:
typeof(D3DCreateBlob) pD3DCreateBlob;                   /* GCC extension */
HRESULT (WINAPI *pD3DCreateBlob)(SIZE_T, ID3DBlob **);  /* Works on both GCC and MSVC */

ch += offset ?: size            /* GCC extension */
ch += offset ? offset : size    /* Works on both GCC and MSVC */
  • When declaring function pointers, put the calling convention inside the parentheses:
HRESULT WINAPI (*pD3DCreateBlob)(SIZE_T, ID3DBlob **);  /* Works on GCC but not MSVC */
HRESULT (WINAPI *pD3DCreateBlob)(SIZE_T, ID3DBlob **);  /* Works on both GCC and MSVC */
  • Use types with dependable definitions:
long wrong = 0xcafe1234l;      /* long is 4 bytes in Win64 but 8 bytes in Unix64 */
LONG right = 0xcafe1234;       /* LONG is 4 bytes, the same as int, on both platforms */
  • Use WCHAR and array syntax for Unicode string literals in non-PE modules (helpful tool):
const WCHAR str1[] = L"Hello";  /* Preferred on PE modules. But it won't compile on non-PE modules. */
const WCHAR str2[] = {          /* Tedious, but correct */
    'H','e','l','l','o',0
};


There is no strict minimum compiler version. In 2023, Wine's chief maintainer Alexandre Julliard gave the following explanation of Wine's compiler support policy:

Our policy is to not draw arbitrary lines of supported/unsupported. That's true for C standard versions, compiler versions, distros, window managers, graphics cards, etc.

We want to support what our users are actually using. If someone reports an issue with their compiler, we want to look into it. We are not going to reject a report because the compiler is "too old", or out of maintenance, or on a distro we've hever heard of.

That doesn't mean we have to fix everything, there's always a trade-off between the complexity/ugliness of the fix, the number of affected users, the availability of a workaround, etc. But that should be decided based on the actual issue, not on some arbitrary version number cutoff.

So it may be that we want to require support for typedef redefinitions, because avoiding them is ugly. In practice, that would mean that you can't build with gcc 4.3 out of the box. But it does not mean that gcc 4.3 is somehow "unsupported". If someone reports some other issue with gcc 4.3, and that one is easy to fix, we'll still want to fix it.

Quality Assurance

(Or, “How do I get Alexandre to apply my patch quickly so I can build on it and it will not go stale?”)

Make sure your patch applies to the current Git HEAD revisions. If a bunch of patches are committed that may affect whether your patch will apply cleanly then verify that your patch does apply! git fetch; git rebase origin is your friend!

Patches must not break building Wine or cause test failures. Adding dead code should also be avoided.

Save yourself some embarrassment and run your patched code against more than just your current test example. Experience will tell you how much effort to apply here. If there are any conformance tests for the code you're working on, run them and make sure they still pass after your patch is applied. Running tests can be done by running make test. You may need to run make testclean to undo the results of a previous test run. See the “testing” guide for more details on Wine conformance tests.

Porting Wine to new Platforms

This document provides a few tips on porting Wine to your favorite (UNIX-based) operating system.

Why ifdef MyOS is probably a mistake.

Operating systems change. Maybe yours doesn't have the foo.h header, but maybe a future version will have it. If you want to #include <foo.h>, it doesn't matter what operating system you are using; it only matters whether foo.h is there.

Furthermore, operating systems change names or “fork” into several ones. An #ifdef MyOS will break over time.

If you use the feature of autoconf -- the Gnu auto-configuration utility -- wisely, you will help future porters automatically because your changes will test for features, not names of operating systems. A feature can be many things:

  • existence of a header file
  • existence of a library function
  • existence of libraries
  • bugs in header files, library functions, the compiler, ...

You will need GNU autoconf, which you can get from your friendly GNU mirror. This program takes Wine configure.ac file and produces a configure shell script that users use to configure Wine to their system.

There are exceptions to the “avoid #ifdef MyOS” rule. Wine, for example, needs the internals of the signal stack -- that cannot easily be described in terms of features. Moreover, you cannot use autoconf HAVE_* symbols in Wine headers, as these may be used by Winelib users who may not be using a configure script.

Let's now turn to specific porting problems and how to solve them.

MyOS doesn't have the foo.h header!

This first step is to make autoconf check for this header. In configure.ac you add a segment like this in the section that checks for header files (search for “header files”):

AC_CHECK_HEADER(foo.h, AC_DEFINE(HAVE_FOO_H))

If your operating system supports a header file with the same contents but a different name, say bar.h, add a check for that also.

Now you can change

#include <foo.h>

to

#ifdef HAVE_FOO_H
#include <foo.h>
#elif defined (HAVE_BAR_H)
#include <bar.h>
#endif

If your system doesn't have a corresponding header file even though it has the library functions being used, you might have to add an #else section to the conditional. Avoid this if you can.

You will also need to add #undef HAVE_FOO_H (etc.) to include/config.h.in.

Finish up with autoconf and ./configure.

MyOS doesn't have the bar function!

A typical example of this is the memmove function. To solve this problem you would add memmove to the list of functions that autoconf checks for. In configure.ac you search for AC_CHECK_FUNCS and add memmove (you will notice that someone already did this for this particular function).

Secondly, you will also need to add #undef HAVE_BAR to include/config.h.in.

The next step depends on the nature of the missing function.

Case 1:

It's easy to write a complete implementation of the function (memmove belongs to this case).

You add your implementation in misc/port.c surrounded by #ifndef HAVE_MEMMOVE and #endif.

You might have to add a prototype for your function. If so, include/miscemu.h might be the place. Don't forget to protect that definition by #ifndef HAVE_MEMMOVE and #endif also!

Case 2:

A general implementation is hard, but Wine is only using a special case.

An example is the various wait calls used in SIGNAL_child from loader/signal.c. Here we have a multi-branch case on features:

#ifdef HAVE_THIS
...
#elif defined (HAVE_THAT)
...
#elif defined (HAVE_SOMETHING_ELSE)
...
#endif

Note that this is very different from testing on operating systems. If a new version of your operating systems comes out and adds a new function, this code will magically start using it.

Finish up with autoconf and ./configure.

Adding New Languages

This section documents the procedure for adding a new language to the list of languages that Wine can display system menus and forms in. Adding new translations is not hard as it requires no programming knowledge or special skills.

First add the language ID (most often like xx, e.g. fr for French, or xx_YY for country-specific variations, e.g. pt_BR for Brazilian Portuguese ; look in tools/wmc/po.c for a list) in the LINGUAS section of configure.ac, and in po/LINGUAS.

Language dependent resources reside in .po files in the po/ directory. Simply create the language-specific .po file using the wine.pot template:

cp po/wine.pot po/xx.po

and start translating (note the file must use UTF-8 encoding).

When adding a new language, also make sure the parameters defined in dlls/kernel32/nls/*.nls fit your local habits and language.

This page was last edited on 22 November 2023, at 01:38.