Thursday, January 17, 2019

Working on Wine Part 5 Fixing Wine

Andrew Eikum a CodeWeavers employee and Wine developer is writing a series of post to introduce people to Wine usage and development.

About This Guide

This is a series of guides intended to introduce software developers to the Wine ecosystem. It will cover what Wine is, how to use Wine, how to debug Wine, how to fix Wine, and what to do with your fix once you've made it.

The guide will be published throughout January.

  • Part 1 describes what Wine is and provides a short description of various popular forks of Wine.
  • Part 2 describes Wine's build process.
  • Part 3 describes how to use Wine as a developer.
  • Part 4 describes how to debug Wine in general.
  • Part 5 describes Wine's source tree layout and how to edit the source.
  • Part 6 describes how you can send your work upstream.

Once you've got some idea of what is causing the problem with your application, it's time to go understand how those APIs are implemented in Wine so you can fix it. You may also need to change Wine's code to debug the application in the first place.


Search for existing bugs

Before digging into the source, it's always useful to first search for existing bugs. You can search Wine's Bugzilla for your application, or for the symptoms you're seeing. As you begin to debug your issue, you may find other search terms to try. If you find a bug, you may find some useful analysis or work has already been done for the issue.

You may also want to look through the Wine Staging patches, especially once you have some idea of what the problem is. You may find someone has already written a patch to fix this problem, but it isn't ready to go upstream yet. You can pick up the torch and try to upstream the work.


Wine source layout

Wine implements the Windows operating system, which is composed of libraries, programs, and kernel functionality. You can find the code for Wine's implementation of each of those components in dlls, programs, and server, respectively. To give you some idea of how different components of Wine work together, some important, core components are listed here.
  • dlls/ntdll – This is where many of the core OS APIs are implemented, like file handling, thread creation and synchronization, timing, and much more. Applications typically use kernel32 instead of ntdll directly.
  • dlls/kernel32 – This is the application-facing interface for the core ntdll APIs mentioned above.
  • dlls/user32 – This is where much of the Windows GUI handling lives, like window creation, message handling, terminal services, and so on.
  • dlls/winex11.drv and dlls/winemac.drv – These libraries map the Windows GUI interfaces, and some other platform-specific functions, to the native platform. user32 calls into these platform drivers.
  • dlls/d3d* and dlls/wined3d – These libraries implement the Direct3D graphics API on top of the platform's OpenGL implementation.
  • programs/services – This program manages background services, both those created by Wine and those provided by installed applications.
  • programs/wineboot – This program kicks off initial prefix creation and other tasks when a prefix is booted.
  • programs/winecfg – This is Wine's configuration program.
  • server – The Wine server implements all cross-process functionality, including message routing, multi-process synchronization primitives, registry handling, and much more. The Wine server is always running if an application is running.
While debug channels are often named after the component in which they are declared (e.g. the dsound channel is declared in dlls/dsound), this is not always the case. You can use git grep to find the relevant debug trace line if it isn't obvious.


Writing tests

In most cases, your work should be driven by tests. Study the documentation for the relevant APIs and write tests first to confirm that Windows's implementation behaves how you expect it to. Then, change Wine's source to pass the tests that you wrote.

Wine has an extensive suite of unit tests. The unit tests comprise well over one million lines of code as of this writing. Any patch you submit must pass all of these tests. Patches should contain more tests proving that the new behavior is correct.

In general, you should focus your work on what a real application actually does. That is, you don't need to implement a bunch of unrelated functionality if the application doesn't actually need it. You should write tests to show how an application is using an API, and how Wine fails to meet its expectations. Then fix Wine to also pass those tests without breaking any existing tests. Any change to Wine has the potential to break other applications. Simply passing tests that you wrote is insufficient reason to change Wine.

You can find the tests for a given component in the tests subdirectory. For example, the tests for dsound live in dlls/dsound/tests. You will find .c files in that directory. The tests start in the START_TEST function and exercise the component being tested. The ok function demonstrates correct behavior. Again, "correct" is defined to mean "like Windows." For example:

    hr = IDirectSound_CreateSoundBuffer(ds, &bufdesc, &primary, NULL);
    ok(hr == S_OK, "CreateSoundBuffer failed: %08x\n", hr);
Here you can see the return value from IDirectSound::CreateSoundBuffer is expected to be S_OK. If it is not S_OK, then the test will fail.

Your tests should demonstrate how your application behaves when interacting with that API. Since you are fixing a bug in Wine's implementation of the API, your tests should demonstrate that Wine will fail without your fix. It isn't necessary, or even recommended, to fully exercise all inputs to an API in the Wine tests. Instead, be thoughtful about how your application might use the API given different inputs from the user, and write tests that reflect those possibilities.

Your tests should be self-contained. They should initialize the necessary functions, test the APIs, and then clean up before continuing. If you need to create files on disk, create them in a temporary location and/or clean them up afterwards. If your test may leave changes to the system, expect to have to clean up after a previous run that crashed, before you run your tests.

After you have written some tests, you need to verify that they pass when run on Windows. To do this locally, run make crosstest, which will use your system's mingw-w64 compiler to build a Windows executable. Run this executable on Windows in the command prompt to verify Windows's behavior. If you do not have a Windows system or VM available, you can upload your patch or test binary to the Wine Test Bot.

Once your tests pass on Windows, run them in Wine with make test. If your tests meaningfully demonstrate a Wine bug, they should fail. Now it is time to fix Wine to pass those tests, as well as all existing tests.


Working with Wine's source

As you develop your fix, there are some rules you should know if you intend to send your work upstream.

For better or worse, Wine does not have a coding style standard and likely never will. The rule is to try to match the surrounding code as best as possible. Especially in older code, you will find a horrible mish-mash of tabs and spaces and brace styles. Do your best to make the code no worse than it was. However, extraneous changes are discouraged as they make review difficult and looking up the source history in Git more cumbersome. If you are changing a line, or even one line in a short code block, it can be re-formatted to be less ugly than it was. But don't reformat an entire function to fit some style, no matter how ugly it is.

Wine only accepts code that adheres to the C89 standard, because some compilers that Wine cares about don't support anything later. The three most common snags here are that your code must declare its variables at the top of the block, you cannot use //-style comments, and function declarations with no arguments must be declared with void arguments:

    int some_function(void);
Resist the urge to gut an entire function or module and re-write it. Small, discrete changes that can be easily understood are the right way to fix Wine. Wine is more than two decades old. There is a lot of hard-won knowledge in much of Wine's source, and throwing out all of that history because it's easier than working in the existing code is not likely to pass review.

Make your changes as obviously correct as possible. Unrelated changes must be placed into their own commits. You should not introduce unused code in an early patch, which begins to be used in a later patch.

Write your patches with the reviewer in mind. It may be unintuitive, but understand that it is more important for your patch to be easy to review than make Wine's code perfect. Reviewer time is one of Wine's most valuable assets. Patches that are easier to review are more likely to pass that review.
Be aware that some tests are "flaky." This is an unfortunate reality of a system as complex as Wine. Ideally all tests would pass on all Windows and Wine environments, but due to bugs, differing platform behavior (especially window managers) and timing differences, they don't. Most modules have well-written tests that should always pass. Some, like the user32 messaging tests, and some audio and graphics tests, fail with some frequency, even on Windows.

Full Article

Run Microsoft Windows Applications and Games on Mac, Linux or ChromeOS save up to 20% off  CodeWeavers CrossOver+ today.

No comments: