PVS-Studio et intégration continue: TeamCity. Ouvrir l'analyse du projet RollerCoaster Tycoon 2

L'un des scénarios les plus pertinents pour l'utilisation de l'analyseur PVS-Studio est son intégration avec les systèmes CI. Et bien que l'analyse d'un projet PVS-Studio à partir de presque n'importe quel système d'intégration continue puisse être intégrée à seulement quelques commandes, nous continuons à rendre ce processus encore plus pratique. PVS-Studio prend désormais en charge la conversion de la sortie de l'analyseur au format TeamCity - TeamCity Inspections Type. Voyons voir comment ça fonctionne.

Informations sur le logiciel utilisé

PVS-Studio est un analyseur statique de code , ++, C # et Java conçu pour faciliter la tâche de recherche et de correction de divers types d'erreurs. L'analyseur peut être utilisé sous Windows, Linux et macOS. Dans cet article, nous utiliserons activement non seulement l'analyseur lui-même, mais également certains utilitaires de son kit de distribution.



CLMonitor est un serveur de surveillance qui surveille les lancements du compilateur. Il doit être exécuté immédiatement avant de commencer à créer votre projet. En mode moniteur, le serveur interceptera les exécutions de tous les compilateurs pris en charge. Il est à noter que cet utilitaire ne peut être utilisé que pour analyser des projets C / C ++.



PlogConverter est un utilitaire pour convertir le rapport de l'analyseur en différents formats.

Informations sur le projet étudié

Essayons cette fonctionnalité avec un exemple pratique - analysons le projet OpenRCT2.



OpenRCT2 est une implémentation open source de RollerCoaster Tycoon 2 (RCT2), en l'étendant avec de nouvelles fonctionnalités et des corrections de bogues. Le gameplay tourne autour de la construction et de l'entretien d'un parc d'attractions, qui abrite des attractions, des boutiques et des installations. Le joueur doit essayer de faire des profits et de maintenir une bonne réputation pour le parc tout en gardant les invités heureux. OpenRCT2 vous permet de jouer à la fois dans un scénario et dans un bac à sable. Les scénarios exigent que le joueur accomplisse une tâche spécifique à un moment donné, tandis que le bac à sable permet au joueur de construire un parc plus flexible sans aucune restriction ni aucun budget.

Personnalisation

Afin de gagner du temps, je vais probablement sauter le processus d'installation et commencer à partir du moment où le serveur TeamCity est en cours d'exécution sur mon ordinateur. Nous devons aller à: localhost: {le port spécifié lors de l'installation} (dans mon cas, localhost: 9090) et entrer les données d'autorisation. Après être entré, nous serons accueillis par:





Cliquez sur le bouton Créer un projet. Ensuite, sélectionnez Manuellement, remplissez les champs.





Après avoir cliqué sur le bouton Créer , nous sommes accueillis par une fenêtre avec des paramètres.





Cliquez sur Créer une configuration de construction .





Remplissez les champs, cliquez sur Créer . Nous voyons une fenêtre proposant de choisir un système de contrôle de version. Étant donné que les sources sont déjà localisées localement, cliquez sur Ignorer .





Enfin, nous passons aux paramètres du projet.





Ajoutez des étapes de construction en cliquant sur: Étapes de construction -> Ajouter une étape de construction .





Ici nous sélectionnons:

• Type de coureur -> Ligne de commande
• Exécuter -> Script personnalisé

Étant donné que nous analyserons lors de la compilation du projet, l'assemblage et l'analyse devraient être une étape, alors remplissez le champ Script personnalisé :





Nous nous attarderons sur les étapes individuelles plus tard. Il est important que le chargement de l'analyseur, la construction du projet, son analyse, la sortie du rapport et le formatage ne prennent que onze lignes de code.



La dernière chose que nous devons faire est de définir les variables d'environnement, que j'ai indiquées de certaines manières pour améliorer leur lisibilité. Pour ce faire, allez dans: Paramètres -> Ajouter un nouveau paramètre et ajoutez trois variables:





Il reste à cliquer sur le bouton Exécuter dans le coin supérieur droit. Pendant que le projet est assemblé et analysé, je vais vous parler du script.

Script directement

Tout d'abord, nous devons télécharger la dernière distribution PVS-Studio. Pour cela, nous utilisons le gestionnaire de paquets hocolatey. Pour ceux qui veulent en savoir plus à ce sujet, il existe un article connexe :

choco install pvs-studio -y

Ensuite, lançons l'utilitaire de suivi d'assemblage de projet CLMonitor.

%CLmon% monitor –-attach

Ensuite, nous allons construire le projet, le chemin vers la version de MSBuild que je dois construire est utilisé comme variable d'environnement MSB

%MSB% %ProjPath% /t:clean
%MSB% %ProjPath% /t:rebuild /p:configuration=release
%MSB% %ProjPath% /t:g2
%MSB% %ProjPath% /t:PublishPortable

Entrons le login et la clé de licence PVS-Studio:

%PVS-Studio_cmd% credentials --username %PVS_Name% --serialNumber %PVS_Key%

Une fois la construction terminée, exécutez à nouveau CLMonitor pour générer des fichiers prétraités et une analyse statique:

%CLmon% analyze -l "c:\ptest.plog"

Ensuite, nous utiliserons un autre utilitaire de notre kit de distribution. PlogConverter convertit le rapport du format standard au format spécifique à TeamCity. Grâce à cela, nous pourrons le voir directement dans la fenêtre d'assemblage.

%PlogConverter% "c:\ptest.plog" --renderTypes=TeamCity -o "C:\temp"

La dernière étape consiste à sortir le rapport formaté vers stdout , où il sera récupéré par l'analyseur TeamCity.

type "C:\temp\ptest.plog_TeamCity.txt"

Code de script complet:

choco install pvs-studio -y
%CLmon% monitor --attach
set platform=x64
%MSB% %ProjPath% /t:clean
%MSB% %ProjPath% /t:rebuild /p:configuration=release
%MSB% %ProjPath% /t:g2
%MSB% %ProjPath% /t:PublishPortable
%PVS-Studio_cmd% credentials --username %PVS_Name% --serialNumber %PVS_Key%
%CLmon% analyze -l "c:\ptest.plog"
%PlogConverter% "c:\ptest.plog" --renderTypes=TeamCity -o "C:\temp"
type "C:\temp\ptest.plog_TeamCity.txt"

En attendant, l'assemblage et l'analyse du projet sont terminés avec succès, nous pouvons aller dans l'onglet Projets et vérifier cela.





Cliquez maintenant sur Inspections Total pour aller voir le rapport de l'analyseur:





Les avertissements sont regroupés par numéros de règle de diagnostic. Pour naviguer dans le code, vous devez cliquer sur le numéro de ligne avec l'avertissement. Cliquez sur le point d'interrogation dans le coin supérieur droit pour ouvrir un nouvel onglet de documentation pour vous. Vous pouvez également naviguer dans le code en cliquant sur le numéro de ligne d'avertissement de l'analyseur. La navigation depuis un ordinateur distant est possible à l'aide du marqueur SourceTreeRoot . Ceux qui sont intéressés par ce mode de fonctionnement de l'analyseur peuvent se familiariser avec la section correspondante de la documentation .

Affichage des résultats de l'analyseur

Une fois que nous avons fini de déployer et de configurer la construction, je suggère de regarder quelques avertissements intéressants trouvés dans le projet à l'étude.



Avertissement N1



V773 [CWE-401] L'exception a été levée sans relâcher le pointeur 'result'. Une fuite de mémoire est possible. libopenrct2 ObjectFactory.cpp 443

Object* CreateObjectFromJson(....)
{
  Object* result = nullptr;
  ....
  result = CreateObject(entry);
  ....
  if (readContext.WasError())
  {
    throw std::runtime_error("Object has errors");
  }
  ....
}

Object* CreateObject(const rct_object_entry& entry)
{
  Object* result;
  switch (entry.GetType())
  {
    case OBJECT_TYPE_RIDE:
      result = new RideObject(entry);
      break;
    case OBJECT_TYPE_SMALL_SCENERY:
      result = new SmallSceneryObject(entry);
      break;
    case OBJECT_TYPE_LARGE_SCENERY:
      result = new LargeSceneryObject(entry);
      break;
    ....
    default:
      throw std::runtime_error("Invalid object type");
  }
  return result;
}

L'analyseur a remarqué une erreur qui, après l'allocation de mémoire dynamique dans CreateObject , lorsqu'une exception est levée, la mémoire n'est pas effacée; en conséquence, une fuite de mémoire se produit.



Attention N2



V501 Il existe des sous-expressions identiques '(1ULL << WIDX_MONTH_BOX)' à gauche et à droite du '|' opérateur. libopenrct2ui Cheats.cpp 487

static uint64_t window_cheats_page_enabled_widgets[] = 
{
  MAIN_CHEAT_ENABLED_WIDGETS |
  (1ULL << WIDX_NO_MONEY) |
  (1ULL << WIDX_ADD_SET_MONEY_GROUP) |
  (1ULL << WIDX_MONEY_SPINNER) |
  (1ULL << WIDX_MONEY_SPINNER_INCREMENT) |
  (1ULL << WIDX_MONEY_SPINNER_DECREMENT) |
  (1ULL << WIDX_ADD_MONEY) |
  (1ULL << WIDX_SET_MONEY) |
  (1ULL << WIDX_CLEAR_LOAN) |
  (1ULL << WIDX_DATE_SET) |
  (1ULL << WIDX_MONTH_BOX) |  // <=
  (1ULL << WIDX_MONTH_UP) |
  (1ULL << WIDX_MONTH_DOWN) |
  (1ULL << WIDX_YEAR_BOX) |
  (1ULL << WIDX_YEAR_UP) |
  (1ULL << WIDX_YEAR_DOWN) |
  (1ULL << WIDX_DAY_BOX) |
  (1ULL << WIDX_DAY_UP) |
  (1ULL << WIDX_DAY_DOWN) |
  (1ULL << WIDX_MONTH_BOX) |  // <=
  (1ULL << WIDX_DATE_GROUP) |
  (1ULL << WIDX_DATE_RESET),
  ....
};

Peu de personnes, à l'exception d'un analyseur statique, pourraient passer ce test d'attention. Cet exemple de copier-coller est parfait pour cela.



Alertes N3



V703 Il est étrange que le champ 'flags' de la classe dérivée 'RCT12BannerElement' écrase le champ de la classe de base 'RCT12TileElementBase'. Vérifiez les lignes: RCT12.h: 570, RCT12.h: 259. libopenrct2 RCT12.h 570

struct RCT12SpriteBase
{
  ....
  uint8_t flags;
  ....
};
struct rct1_peep : RCT12SpriteBase
{
  ....
  uint8_t flags;
  ....
};

Bien entendu, utiliser une variable du même nom dans la classe de base et dans l'héritier n'est pas toujours une erreur. Cependant, la technologie d'héritage elle-même suppose la présence de tous les champs de la classe parent dans l'enfant. En déclarant un champ avec le même nom dans l'héritier, nous introduisons une confusion.



Avertissement N4



V793 Il est étrange que le résultat de l'instruction 'imageDirection / 8' fasse partie de la condition. Peut-être que cette affirmation aurait dû être comparée à autre chose. libopenrct2 ObservationTower.cpp 38

void vehicle_visual_observation_tower(...., int32_t imageDirection, ....)
{
  if ((imageDirection / 8) && (imageDirection / 8) != 3)
  {
    ....
  }
  ....
}

Regardons de plus près. L'expression imageDirection / 8 sera fausse si imageDirection est comprise entre -7 et 7. La deuxième partie: (imageDirection / 8)! = 3 vérifie que l' imageDirection est hors de portée: -31 à -24 et 24 à 31 respectivement. Il me semble assez étrange de vérifier les nombres pour entrer une certaine plage de cette manière, et même s'il n'y a pas d'erreur dans ce fragment de code, je recommanderais de réécrire ces conditions pour être plus explicite. Cela rendrait la vie beaucoup plus facile pour les personnes qui liraient et maintiendraient ce code.



Avertissement N5



V587Une étrange séquence d'affectations de ce type: A = B; B = A;. Vérifiez les lignes: 1115, 1118.libopenrct2ui MouseInput.cpp 1118

void process_mouse_over(....)
{
  ....
  switch (window->widgets[widgetId].type)
  {
    case WWT_VIEWPORT:
      ebx = 0;
      edi = cursorId;                                 // <=
      // Window event WE_UNKNOWN_0E was called here,
      // but no windows actually implemented a handler and
      // it's not known what it was for
      cursorId = edi;                                 // <=
      if ((ebx & 0xFF) != 0)
      {
        set_cursor(cursorId);
        return;
      }
      break;
      ....
  }
  ....
}

Ce morceau de code a très probablement été obtenu par décompilation. Ensuite, à en juger par le commentaire laissé, une partie du code non fonctionnel a été supprimée. Cependant, il reste quelques opérations sur cursorId , qui n'ont pas non plus beaucoup de sens.



N6 Avertissement



V1004 [CWE-476] Le pointeur 'player' a été utilisé de manière non sécurisée après avoir été vérifié par rapport à nullptr. Vérifiez les lignes: 2085, 2094.libopenrct2 Network.cpp 2094

void Network::ProcessPlayerList()
{
  ....
  auto* player = GetPlayerByID(pendingPlayer.Id);
  if (player == nullptr)
  {
    // Add new player.
    player = AddPlayer("", "");
    if (player)                                          // <=
    {
      *player = pendingPlayer;
       if (player->Flags & NETWORK_PLAYER_FLAG_ISSERVER)
       {
         _serverConnection->Player = player;
       }
    }
    newPlayers.push_back(player->Id);                    // <=
  }
  ....
}

Il est assez simple de corriger ce code, vous devez soit vérifier le lecteur pour un pointeur nul une troisième fois , soit l'ajouter au corps de l'opérateur conditionnel. Je suggérerais la deuxième option:

void Network::ProcessPlayerList()
{
  ....
  auto* player = GetPlayerByID(pendingPlayer.Id);
  if (player == nullptr)
  {
    // Add new player.
    player = AddPlayer("", "");
    if (player)
    {
      *player = pendingPlayer;
      if (player->Flags & NETWORK_PLAYER_FLAG_ISSERVER)
      {
        _serverConnection->Player = player;
      }
      newPlayers.push_back(player->Id);
    }
  }
  ....
}

N7 Avertissement



V547 [CWE-570] L'expression 'nom == nullptr' est toujours fausse. libopenrct2 ServerList.cpp 102

std::optional<ServerListEntry> ServerListEntry::FromJson(...)
{
  auto name = json_object_get(server, "name");
  .....
  if (name == nullptr || version == nullptr)
  {
    ....
  }
  else
  {
    ....
    entry.name = (name == nullptr ? "" : json_string_value(name));
    ....
  }
  ....
}

Vous pouvez vous débarrasser de la ligne de code difficile à lire d'un seul coup et résoudre le problème de vérification de nullptr . Je suggère de changer le code comme suit:

std::optional<ServerListEntry> ServerListEntry::FromJson(...)
{
  auto name = json_object_get(server, "name");
  .....
  if (name == nullptr || version == nullptr)
  {
    name = ""
    ....
  }
  else
  {
    ....
    entry.name = json_string_value(name);
    ....
  }
  ....
}

Avertissement N8



V1048 [CWE-1164] La variable 'ColumnHeaderPressedCurrentState' a reçu la même valeur. libopenrct2ui CustomListView.cpp 510

void CustomListView::MouseUp(....)
{
  ....
  if (!ColumnHeaderPressedCurrentState)
  {
    ColumnHeaderPressed = std::nullopt;
    ColumnHeaderPressedCurrentState = false;
    Invalidate();
  }
}

Le code a l'air assez bizarre. Je pense qu'il y avait une faute de frappe ou être soumis à, ou lors de la réaffectation de la valeur de la variable ColumnHeaderPressedCurrentState à false .

Production

Comme nous pouvons le voir, l'intégration de l'analyseur statique PVS-Studio dans votre projet TeamCity est assez simple. Il suffit d'écrire un seul petit fichier de configuration pour cela. La révision du code vous permettra d'identifier les problèmes immédiatement après la construction, ce qui aidera à les éliminer lorsque la complexité et le coût des modifications sont encore faibles.

Si vous souhaitez partager cet article avec un public anglophone, veuillez utiliser le lien de traduction: Vladislav Stolyarov. PVS-Studio et intégration continue: TeamCity. Analyse du projet Open RollerCoaster Tycoon 2 .