これはSigillシグナルが途中で存在するのはなぜですか？

Question

私は、2人の戦闘員の戦いで勝者の名前を返さなければならないという問題があります。 次のように戦闘機のためのクラスがある：

class Fighter 
{ 
private: 
std::string name; 

int health; 

int damagePerAttack; 

public: 
Fighter(std::string name, int health, int damagePerAttack) 
{ 
    this->name = name; 
    this->health = health; 
    this->damagePerAttack = damagePerAttack; 
} 

~Fighter() { }; 

std::string getName() 
{ 
    return name; 
} 

int getHealth() 
{ 
    return health; 
} 

int getDamagePerAttack() 
{ 
    return damagePerAttack; 
} 

void setHealth(int value) 
{ 
    health = value; 
} 
}; 

が、私は勝者の名前を返すべき関数を書きました。

std::string declareWinner(Fighter* fighter1, Fighter* fighter2, 
    std::string firstAttacker) 
    { 
    // Your code goes here. Have fun! 
    if(firstAttacker==fighter1->getName()) 
    { 
     while(fighter1->getHealth()!=0&&fighter2->getHealth()!=0) 
     { 
     fighter2->setHealth(fighter2->getHealth()-fighter1->getDamagePerAttack()); 
     if(fighter2->getHealth()<=0) 
     { 
      return fighter1->getName(); 
     } 
     fighter1->setHealth(fighter1->getHealth()-fighter2->getDamagePerAttack()); 
     if(fighter1->getHealth()<=0) 
     { 
      return fighter2->getName(); 
     } 
     } 
    } 
    else if(firstAttacker==fighter2->getName()) 
    { 
     while(fighter1->getHealth()!=0&&fighter2->getHealth()!=0) 
     { 
     fighter1->setHealth(fighter1->getHealth()-fighter2->getDamagePerAttack()); 
     if(fighter1->getHealth()<=0) 
     { 
      return fighter2->getName(); 
     } 
     fighter2->setHealth(fighter2->getHealth()-fighter1->getDamagePerAttack()); 
     if(fighter2->getHealth()<=0) 
     { 
      return fighter1->getName(); 
     } 
     } 
    } 
    } 

これは、すべての私のニーズを満たし、それがSIGILL信号をスローし、私は私が間違っていたかわかりません。私はそれをどのように扱うべきですか？

Answer 1

一部の状況では、関数が最後まで実行され、値を返さずに終了し、スタックを破壊し、SIGILLにつながる可能性があります。安全な手段として、たとえば、関数の最後にreturnステートメントを追加することができます。

std::string declareWinner(Fighter* fighter1, Fighter* fighter2, 
    std::string firstAttacker) 
{ 
    // Your code goes here. Have fun! 
     if(firstAttacker==fighter1->getName()) 
     { 
      while(fighter1->getHealth()!=0&&fighter2->getHealth()!=0) 
      { 
      fighter2->setHealth(fighter2->getHealth()-fighter1->getDamagePerAttack()); 
      if(fighter2->getHealth()<=0) 
      { 
       return fighter1->getName(); 
      } 
      fighter1->setHealth(fighter1->getHealth()-fighter2->getDamagePerAttack()); 
      if(fighter1->getHealth()<=0) 
      { 
       return fighter2->getName(); 
      } 
      } 
     } 
     else if(firstAttacker==fighter2->getName()) 
     { 
      while(fighter1->getHealth()!=0&&fighter2->getHealth()!=0) 
      { 
      fighter1->setHealth(fighter1->getHealth()-fighter2->getDamagePerAttack()); 
      if(fighter1->getHealth()<=0) 
      { 
       return fighter2->getName(); 
      } 
      fighter2->setHealth(fighter2->getHealth()-fighter1->getDamagePerAttack()); 
      if(fighter2->getHealth()<=0) 
      { 
       return fighter1->getName(); 
      } 
      } 
     } 
     return "No winner"; <= Add before exiting function 
} 

また、冗長性とコード内での論理エラーが考えられます。 私は（関数のシグネチャを変更せずに）このようにそれを書き直します：

std::string declareWinner(Fighter* fighter1, Fighter* fighter2, 
     std::string firstAttacker) 
{ 
    Fighter *first; 
    Fighter *second; 
    if(firstAttacker == fighter1->getName()) { 
     first = fighter2; 
     second = fighter1; 
    } else if (firstAttacker == fighter2->getName()) { 
     first = fighter1; 
     second = fighter2; 
    } else { 
     // Bad call parameters 
     return "Bad call"; // Throw exception maybe? 
    } 
    // Simulating fighting 
    do { 
     std::swap(second,first); 
     second->setHealth(second->getHealth() - first->getDamagePerAttack()); 
    } while (second->getHealth() > 0); 
    return first->getName(); 

}