Forum Programmation.c Sonar Qube faux positif…

Posté par  . Licence CC By‑SA.
Étiquettes : aucune
0
26
fév.
2020

Bonjour,

J’ai un soucis, sonarqube me dit que je peux écrire en dehors d’un tableau… Je vous montre le code.

   condition = (index < taille_max);

   checkCondition(condition);

   (appelDeHook[index])(parametres);

En gros il me dit que si index >= taille_max, alors appelDeHook[index] tape à l’extérieur du tableau. Ce qui n’est pas faux, sauf que checkCondition est implémentée comme suis :

void checkCondition(int condition)
{
    if (!condition)
    {
         resetBoard();
    }
}

Donc si la condition est fausse, la carte redémarre. Je ne peux donc pas aller en dehors du tableau. Comment puis-je dire à Sonar que si condition==false => checkCondition <=> exit ?

Je peux dire à Sonar que c’est un faux positif, mais si ma fonction change, ou autre, me reproposera-t-il l’erreur ?

  • # Sonar n’a pas tort…

    Posté par  . Évalué à 1. Dernière modification le 27 février 2020 à 00:05.

    Que fait la fonction resetBoard() ? Elle change la taille du tableau de pointeurs de fonctions appelDeHook[] ? Mais de toute manière, comme elle ne prend pas en paramètre l’index mis en cause, il est certain qu’elle ne peut pas te prémunir du dépassement en élargissant suffisamment le tableau !

    Et quand bien même, ce tableau, il faut le remplir avec des pointeurs valides, donc j’imagine que tu ne pourras pas l’étendre autant que nécessaire pour éviter le dépassement.

    Bref, Sonar n’a pas tort, et il va falloir en tenir compte. Qu’est-ce que tu veux faire, exactement ? Quel est le comportement attendu si l’index a une valeur négative ou trop grande ? On ne tient pas compte de l’appel ? On exécute appelDeHook[0] avec les paramètres ? On fait encore autre chose ?

    • [^] # Re: Sonar n’a pas tort…

      Posté par  . Évalué à 1. Dernière modification le 27 février 2020 à 00:37.

      Au temps pour moi ; c’est de l’embarqué, et tu redémarres tout si l’index est trop grand, c’est bien ça ? Désolé pour ma réponse à côté de la plaque. :)

      Du coup, la solution est vite vue :

      if (index < taille_max)
    appelDeHook[index](parametres);
else
    resetBoard();

      Et Sonar sera content ! Voilou. Bon, c’est un peu brutal pour l’utilisateur de la carte, qui va payer cher les erreurs logicielles, mais ce n’est pas le problème ici, et je n’ai pas tous les éléments pour faire ce genre de réflexions.

      • [^] # Re: Sonar n’a pas tort…

        Posté par  . Évalué à 2.

        Je suis bien d’accord, mais je reprends un code, il y en a pas loin de 280, et il y a trop de cas pour faire un sed ou awk en automatique.

        • [^] # Re: Sonar n’a pas tort…

          Posté par  . Évalué à 1.

          Si tu veux corriger tout ça de manière automatique sur, mettons, 280 cas, tu peux, plus simplement, remplacer tous les

          checkCondition(condition);

          par

          if (!condition)
    resetBoard();
else

          Avec la petite tabulation à la fin pour garder une indentation correcte.

          • [^] # Re: Sonar n’a pas tort…

            Posté par  . Évalué à 1.

            Hélas non, parfois j’ai des trucs comme ça :

              condition = (…);
  checkCondition(condition);
  condition = (…bis);
  checkCondition(condition);
  …

            Là ça devient compliqué, j’avais pensé faire :

            #define checkCondition(cond) \
   if (!cond) \
   { \
       resetBoard(); \
       return; \
   }

            Là, sonar détecterait qu’on ne revient pas, même si le return est inatteignable.
            Mais on a le droit qu’à un seul return par fonction… Saleté de norme SIL, DAL, etc.

            • [^] # Re: Sonar n’a pas tort…

              Posté par  . Évalué à 2. Dernière modification le 27 février 2020 à 11:47.

              Et si on fait ça,

              int checkCondition(condition)
{
    if (!condition)
    {
        resetBoard();
    }

    return condition;
}

int ok = 1;
cond = a;
ok = ok && checkCondition(cond);
cond = b;
ok = ok && checkCondition(cond);

if (ok)
{
    appelDeHook[index](parametres);
}

              Sonar est assez malin pour que ça passe ?

              • [^] # Re: Sonar n’a pas tort…

                Posté par  . Évalué à 2.

                C’est pas mal, je vais voir si je peux tester. Il faut quand même faire les if (ok) manuellement, car il peut y avoir pas mal d’instruction.

                Sinon, le SIL impose « pas d’appel de fonction dans les opérations booléenne sauf sur le premier élément » car on ne sait pas si la fonction est appelée :'(

                Mais ils acceptent l’inverse :

                ok = checkCondition(condition) && ok;

                L’idéal aurait été d’avoir un truc comme ça.

                int checkCondition(int condition)
{
   if (!condition)
   {
       // SONAR: exit software.
       resetBoard();
   }
}

                • [^] # Re: Sonar n’a pas tort…

                  Posté par  . Évalué à 4.

                  on a le droit qu’à un seul return par fonction… Saleté de norme SIL, DAL, etc.

                  le SIL impose « pas d’appel de fonction dans les opérations booléenne sauf sur le premier élément »

                  Le SIL t'impose d'avoir des règles de codage. Ces règles de codage t'imposent un seul return par fonction ou ne pas appeler de fonction dans des opérations booléennes.

                  Si ce code est dans un logiciel sécuritaire et que tu modifies les 280 occurrences de cette condition, en théorie, il faudrait aussi repasser un paquet de tests ! Bon courage !

                • [^] # Re: Sonar n’a pas tort…

                  Posté par  . Évalué à 2. Dernière modification le 27 février 2020 à 15:04.

                  L’idéal aurait été d’avoir un truc comme ça.

                  Effectivement, ce serait plus propre.
                  J'imagine que tu n'as pas droit non plus au goto ? Je me doute de la réponse, mais sinon :

                  if (!condition)
{
    goto exit_software;
}

exit_software:
    resetBoard();

                  Avant que les puristes me tombent dessus, la gestion des erreurs est un cas d'utilisation classique et propre du goto. ;)

Suivre le flux des commentaires

Note : les commentaires appartiennent à celles et ceux qui les ont postés. Nous n’en sommes pas responsables.