# Ryujinx: re\-checking the Nintendo Switch emulator using PVS\-Studio

Nintendo Switch's popularity hasn't waned, and with exclusive games winning awards, the desire to play them is only growing\. However, not everyone has the opportunity to try out this portable gaming console\. Ryujinx — a Nintendo Switch emulator — solves this problem\. Today we are going to check its code using the PVS\-Studio analyzer\.

![1059_Ryujinx/image1.png](https://import.viva64.com/docx/blog/1059_Ryujinx/image1.png)

## About the project

Ryujinx is a Nintendo Switch emulator\. According to the developers, it aims to provide excellent accuracy, performance, and a user\-friendly interface\. The project is written in C\#\. It's being actively developed and available in the [GitHub repository](https://github.com/Ryujinx/Ryujinx)\.

Among the Nintendo Switch emulators, Ryujinx also has competitors — for example, the Yuzu project written in C\+\+\. It was reviewed in one of our [articles](https://pvs-studio.com/en/blog/posts/cpp/0651/) on project checking\. 

In the PVS\-Studio [blog](https://pvs-studio.com/en/blog/posts/?tag=gamedev), there was also an [article](https://pvs-studio.com/en/blog/posts/csharp/0840/) in which we checked the Ryujinx emulator\. Although, a lot of time has passed since then: many new features have appeared and brought new errors\. Let's talk about them today\.

## Error analysis

### Inattention, typos, copy\-paste

Such mistakes are easy to miss\. If you are lucky, they may not affect the program performance, but otherwise they may change its logic\. 

**Issue 1**

So, let's start with an attentiveness test\.

I'm going to show you a large code fragment\. Can you find a catch there?

```cpp
public static PrimitiveType Convert(this PrimitiveTopology topology)
{
  switch (topology)
  {
    case PrimitiveTopology.Points:
      return PrimitiveType.Points;
    case PrimitiveTopology.Lines:
       return PrimitiveType.Lines;
    case PrimitiveTopology.LineLoop:
       return PrimitiveType.LineLoop;
    case PrimitiveTopology.LineStrip:
       return PrimitiveType.LineStrip;
    case PrimitiveTopology.Triangles:
       return PrimitiveType.Triangles;
    case PrimitiveTopology.TriangleStrip:
       return PrimitiveType.TriangleStrip;
    case PrimitiveTopology.TriangleFan:
       return PrimitiveType.TriangleFan;
    case PrimitiveTopology.Quads:
       return PrimitiveType.Quads;
    case PrimitiveTopology.QuadStrip:
       return PrimitiveType.QuadStrip;
    case PrimitiveTopology.Polygon:
       return PrimitiveType.TriangleFan;
    case PrimitiveTopology.LinesAdjacency:
       return PrimitiveType.LinesAdjacency;
    case PrimitiveTopology.LineStripAdjacency:
       return PrimitiveType.LineStripAdjacency;
    case PrimitiveTopology.TrianglesAdjacency:
       return PrimitiveType.TrianglesAdjacency;
    case PrimitiveTopology.TriangleStripAdjacency:
       return PrimitiveType.TriangleStripAdjacency;
    case PrimitiveTopology.Patches:
       return PrimitiveType.Patches;
  }
  ....
}
```





<details>
   <summary>The PVS\\\-Studio warning:</summary>

[V3139](https://pvs-studio.com/en/docs/warnings/v3139/) Two or more case\-branches perform the same actions\. EnumConversion\.cs 448


</details>


The error is not so difficult to notice if you examine this code fragment closely\. However, would you like to do it while working and not knowing that there is an error? 

If this made your eyes dazzled, I'll give you a hint:

```cpp
switch (topology)
{
  ....
  case PrimitiveTopology.TriangleFan:
    return PrimitiveType.TriangleFan;
  ....
  case PrimitiveTopology.Polygon:
    return PrimitiveType.TriangleFan;
  ....
}
```

This is a classic copy\-paste error\. The second _case_ section should return the _PrimitiveType\.Polygon_ value\. Let's make sure that it's in the _PrimitiveType_ enumeration:

```cpp
public enum PrimitiveType
{
  ....
  TriangleFan = 6,
  ....
  Polygon = 9,
  ....
}
```

Copy\-paste is convenient and fast, but as the fragment gets bigger and bigger, the possibility of making mistakes also grows\. 

**Issue 2**

Let's take a look at a suspicious piece of code:

```cpp
private void YesButton_Clicked(object sender, EventArgs args)
{
  ....
  Window.Functions = _mainWindow.Window.Functions =
    WMFunction.All & WMFunction.Close;
  ....
}
```

The PVS\-Studio warning: [V3182](https://pvs-studio.com/en/docs/warnings/v3182/) The result of 'WMFunction\.All & WMFunction\.Close' expression is '0'\. It is possible that the '\|' operator should be used instead\. UpdateDialog\.cs 69

As we can see from the warning, the analyzer complains about the '&' operator\. To understand why, let's take a look at the _WMFunction_ enumeration:

```cpp
[Flags]
public enum WMFunction
{
  All = 0x1,
  Resize = 0x2,
  Move = 0x4,
  Minimize = 0x8,
  Maximize = 0x10,
  Close = 0x20
}
```

We are dealing with bit flags, but in this case the implementation of combining them doesn't work: the result of the bitwise AND \(&\) operation for the _WMFunction\.All_ and _WMFunction\.Close_ values will be equal to zero\.

The analyzer immediately gives us a hint on how to solve this problem\. The correct implementation of combining flags may look as follows:

```cpp
Window.Functions = _mainWindow.Window.Functions =
  WMFunction.All | WMFunction.Close;
```

**Issue 3**

Another interesting case:

```cpp
private BaseNode ParseSpecialName(....)
{
  switch (....)
  {
    case 'C':
      _position += 2;
      BaseNode firstType = ParseType();
      if (   firstType == null 
          || ParseNumber(true).Length == 0 
          || !ConsumeIf("_"))
      {
        return null;
      }
      BaseNode secondType = ParseType();
      return new CtorVtableSpecialName(secondType, firstType); // <=
  }
}
```

The PVS\-Studio warning: [V3066](https://pvs-studio.com/en/docs/warnings/v3066/) Possible incorrect order of arguments passed to 'CtorVtableSpecialName' constructor: 'secondType' and 'firstType'\. Demangler\.cs 803

The order of arguments looks strange, but let's not guess and check the method:

```cpp
public CtorVtableSpecialName(BaseNode firstType, BaseNode secondType) :
  base(NodeType.CtorVtableSpecialName)
{
  _firstType  = firstType;
  _secondType = secondType;
}
```

At first glance, the order of parameters when called really seems wrong, but a programmer could have done it on purpose\. 

However, how to understand whether the developers intended to write like this or made a mistake? Comments could have helped, however there are none\. 

**Note\.** In addition to the new warnings, there are still unfixed errors from the last check:

* [V3021](https://pvs-studio.com/en/docs/warnings/v3021/) There are two 'if' statements with identical conditional expressions\. The first 'if' statement contains method return\. This means that the second 'if' statement is senseless Demangler\.cs 2043
* [V3013](https://pvs-studio.com/en/docs/warnings/v3013/) It is odd that the body of 'PrintLeft' function is fully equivalent to the body of 'PrintRight' function \(10, line 18\)\. PackedTemplateParameter\.cs 10

You can find the analysis of these errors in this [article](https://pvs-studio.com/en/blog/posts/csharp/0840/)\. 

### NullReferenceException and more

Suspicious interactions with potential null references are a common pattern\. 

**Issue 4**

Let's take a look at the dangerous course of action: 

```cpp
public Result Initialize(....)
{
  ....
  if (type == ThreadType.User)
  {
    if (owner.AllocateThreadLocalStorage(out _tlsAddress) 
        != Result.Success)
    {
      return KernelResult.OutOfMemory;
    }

    MemoryHelper.FillWithZeros(owner.CpuMemory, 
                               _tlsAddress,
                               KTlsPageInfo.TlsEntrySize);
  }

  if (owner != null)
  {
    Owner = owner;
    owner.IncrementReferenceCount();
    owner.IncrementThreadCount();
    ....
  }
  else
  {
    is64Bits = true;
  }
}
```

The PVS\-Studio warning: [V3095](https://pvs-studio.com/en/docs/warnings/v3095/) The 'owner' object was used before it was verified against null\. Check lines: 169, 174\. KThread\.cs 169

We are interested in the _owner_ variable\. It is checked for _null_, but before that, it is used without checking\. This code looks suspicious\. 

The question is: is there an error here, or is there some kind of connection between the _owner_ and _type_ variables, and there will be no exception? It's hard to say from an outside perspective, developers should know better\. Anyway, the code is still suspicious\.

By the way, the analyzer did not work quite correctly, as it pointed to the _owner\.CpuMemory_ access, not _owner\.AllocateThreadLocalStorage_\. Here we have something to work on\. :\)

**Issue 5**

And now vice versa\. Let's take a look at the following code:

```cpp
private void RenderLoop()
{
  ....
  _renderer?.Window?.SetAntiAliasing(....);
  _renderer?.Window?.SetScalingFilter(....);
  _renderer?.Window?.SetScalingFilterLevel(....);
  ....
  _renderer.Window.SetSize(....);
  ....
}
```

The PVS\-Studio warning: [V3125](https://pvs-studio.com/en/docs/warnings/v3125/) The '\_renderer\.Window' object was used after it was verified against null\. Check lines: 882, 877\. AppHost\.cs 882

This time the developer checks _\_renderer\.Window_ using the '?\.' null\-conditional operator, but then, the check is omitted\. 

It's hard to say whether the developer wanted this or just forgot to add the check\.

**Note\.** In addition to the new errors, the project still contains unfixed errors from the last check:

* [V3095](https://pvs-studio.com/en/docs/warnings/v3095/) The 'IManager\.NsdSettings' object was used before it was verified against null\. Check lines: 37, 41\. FqdnResolver\.cs 37
* [V3125](https://pvs-studio.com/en/docs/warnings/v3125/) The 'Owner' object was used after it was verified against null\. Check lines: 1308, 1306\. KThread\.cs 1308
* [V3105](https://pvs-studio.com/en/docs/warnings/v3105/) The 'result' variable was used after it was assigned through null\-conditional operator\. NullReferenceException is possible\. Client\.cs 214

**Issue 6**

Let's look at the following method, which the analyzer found suspicious: 

```cpp
public (uint, bool) GetFormat(....)
{
  TextureSpecializationState state = 
    GetTextureSpecState(stageIndex, handle, cbufSlot).Value;
  ....
}
```

At first glance, the issue is not visible\. However, the devil, as they say, is in the details\. Note the warning:

[V3080](https://pvs-studio.com/en/docs/warnings/v3080/) Possible null dereference of method return value\. Consider inspecting: GetTextureSpecState\(\.\.\.\)\. ShaderSpecializationState\.cs 408

Let's follow the path of the analyzer user and look at the method from the inside:

```cpp
private Box<....> GetTextureSpecState(....)
{
  TextureKey key = new TextureKey(stageIndex, handle, cbufSlot);
  if (_textureSpecialization.TryGetValue(key,
        out Box<TextureSpecializationState> state))
  {
    return state;
  }
  return null;
}
```

Since there is a _null_ return possibility, it's a good idea to add a check\.

It's worth noting that such code occurs several times:

* [V3080](https://pvs-studio.com/en/docs/warnings/v3080/) Possible null dereference of method return value\. Consider inspecting: GetTextureSpecState\(\.\.\.\)\. ShaderSpecializationState\.cs 408
* [V3080](https://pvs-studio.com/en/docs/warnings/v3080/) Possible null dereference of method return value\. Consider inspecting: GetTextureSpecState\(\.\.\.\)\. ShaderSpecializationState\.cs 420
* [V3080](https://pvs-studio.com/en/docs/warnings/v3080/) Possible null dereference of method return value\. Consider inspecting: GetTextureSpecState\(\.\.\.\)\. ShaderSpecializationState\.cs 431

If you want to learn more about _NullReferenceException_, I recommend you reading [this article](https://pvs-studio.com/en/blog/posts/csharp/1049/)\. For example, you can check if you remember all the listed ways of encountering NRE\. 

### null from the past

Unfixed errors of the past caused new ones\.

**Issue 7**

The following warning was described in the previous article about this project check:

```cpp
public void LoadApplication(string path, bool isFirmwareTitle)
{
  ....
  firmwareVersion = _contentManager.GetCurrentFirmwareVersion();
  string message = $"{firmwareVersion.VersionString}.....";
  GtkDialog.CreateInfoDialog($"{firmwareVersion.VersionString} .... ");
  ....
}
```

The PVS\-Studio warning: [V3080](https://pvs-studio.com/en/docs/warnings/v3080/) Possible null dereference\. Consider inspecting 'firmwareVersion'\. MainWindow\.cs 734

The issue is that the _FirmwareVersion_ variable can have the _null _value, but there is no check before it's used\. Let's take a look at the _GetCurrentFirmwareVersion_ method:

```cpp
public SystemVersion GetCurrentFirmwareVersion()
{
  LoadEntries();
  lock (_lock)
  {
    ....
    if (....)
    {
      return new SystemVersion(systemVersionFile.AsStream());
    }
    ....
  }
  return null; // <=
}
```

There is a possibility of returning _null_, which may cause an error\.

In addition to the previously mentioned warning, a new one has emerged: 

```cpp
public async Task<bool> LoadGuestApplication(){
  ....
  firmwareVersion = ContentManager.GetCurrentFirmwareVersion();


  await ContentDialogHelper.CreateInfoDialog(
    LocaleManager.Instance.UpdateAndGetDynamicValue(
      ....,  
      firmwareVersion.VersionString),                  // <=
    LocaleManager.Instance.UpdateAndGetDynamicValue(
      ...., 
      firmwareVersion.VersionString),                  // <=
    ....);               
  ....
}
```

The PVS\-Studio warning: [V3080](https://pvs-studio.com/en/docs/warnings/v3080/) Possible null dereference\. Consider inspecting 'firmwareVersion'\. AppHost\.cs 541

It's worth adding that now there is a null check for _firmwareVersion_ in the code below:

```cpp
public async Task<bool> LoadGuestApplication()
{
  ....
  firmwareVersion = ContentManager.GetCurrentFirmwareVersion();
  ....
  Logger.Notice.Print(
    LogClass.Application, 
    $"Using Firmware Version: {firmwareVersion?.VersionString}");
}
```

## Conclusion

We hope this article will help the developers improve their project\. 

It's worth mentioning that about two years have passed since the last check, but some errors have not been fixed\. We recommend the Ryujinx developers to pay attention to them and read the previous [article](https://pvs-studio.com/en/blog/posts/csharp/0840/)\.

If you are interested in articles about checking game projects, I suggest you reading our [blog](https://pvs-studio.com/en/blog/posts/?tag=gamedev)\.

And if you'd like to check your code using PVS\-Studio, you can download and try it [here](https://pvs-studio.com/en/pvs-studio/try-free/)\.