Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suspicious code fragments found by PVS-Studio #146

Open
AGB-DevRel opened this issue Nov 27, 2023 · 1 comment
Open

Suspicious code fragments found by PVS-Studio #146

AGB-DevRel opened this issue Nov 27, 2023 · 1 comment

Comments

@AGB-DevRel
Copy link

Hello
I found some suspicious fragments in the project source code with the help of the PVS-Studio static analyzer.
A few suspicious code snippets are bellow. Perhaps some of them are worth fixing.

Issue 1

private void OnDrawGizmosSelected()
{
  ....
  var vehicles = FindObjectsOfType<Vehicle>()
                .Where(....)
                .OrderBy(....)
                .ToArray();
  foreach (var vehicle in vehicles)
  {
    foreach (var seat in vehicle.Seats){....}
    var closestSeat = vehicle.GetSeatAlignmentOfClosestSeat(transform.position);
    if (closestSeat != Vehicle.SeatAlignment.None){....}
    break;
  }
}

Link to the sources
A break occurs under any circumstances on the first iteration of the loop. As a result, only the first element from the entire vehicles collection will be used in the loop.

Issue 2

public void SetString(string key, string value)
{
  if (m_syncDictionary.TryGetValue(key, out string existingValue))
  {
    if (value != existingValue)
    {
      m_syncDictionary[key] = value; // <=
    }
  }
  m_syncDictionary[key] = value; // <=
}

Link to the sources
The m_syncDictionary[key] = value assignment occurs regardless of the check results.

@in0finite
Copy link
Owner

Thanks for reporting this.

The first one can obviously be written in a better way (without foreach loop).

The second one is indeed a mistake.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants