📤 ℹ️ 🧖🏾 命令与征服游戏代码：90年代的错误。第二卷 📠 🈯️ ⛹🏼

美国公司Electronic Arts Inc（EA）已发布了《命令与征服：泰伯利亚黎明》和《命令与征服：红色警戒》游戏的开源代码。使用PVS-Studio分析仪在源代码中发现了几十个错误，因此，欢迎继续描述发现的缺陷。

介绍

Command＆Conquer是实时策略类型的一系列计算机游戏。该系列的第一款游戏于1995年发布。游戏的源代码被公布与释放命令与征服修复集合。

为了找到代码中的错误，使用了PVS-Studio分析仪。它是用于检测用C，C ++，C＃和Java编写的程序的源代码中的错误和潜在漏洞的工具。

链接到第一个错误摘要：“命令与征服游戏：90年代的错误。第一卷”。

条件错误

V583'？：'运算符，无论其条件表达式如何，始终返回一个相同的值：3072。STARTUP.CPP 1136

void Read_Setup_Options( RawFileClass *config_file )
{
  ....
  ScreenHeight = ini.Get_Bool("Options", "Resolution", false) ? 3072 : 3072;
  ....
}

事实证明，用户无法影响某些设置。更准确地说，他们做了一些事情，但是由于三元运算符总是返回一个值，因此实际上没有任何变化。

V590考虑检查'i <8 && i <4'表达式。表达式过多或打印错误。 DLLInterface.cpp 2238

// Maximum number of multi players possible.
#define MAX_PLAYERS 8 // max # of players we can have

for (int i = 0; i < MAX_PLAYERS && i < 4; i++) {
  if (GlyphxPlayerIDs[i] == player_id) {
    MultiplayerStartPositions[i] = XY_Cell(x, y);
  }
}

由于周期错误，因此并未为所有玩家设置位置。一方面，我们看到常数MAX_PLAYERS 8，并假设这是最大的玩家数量。另一方面，我们看到条件<4和&&运算符。因此，循环永远不会进行8次迭代。最有可能的是，在开发的初始阶段，程序员没有使用常量，而当他开始时，他忘记了从代码中删除旧数字。

V648“ &&”操作的优先级高于“ ||”的优先级操作。婴儿CPP 1003

void InfantryClass::Assign_Target(TARGET target)
{
  ....
  if (building && building->Class->IsCaptureable &&
    (GameToPlay != GAME_NORMAL || *building != STRUCT_EYE && Scenario < 13)) {
    Assign_Destination(target);
  }
  ....
}

通过不为||运算符指定操作优先级，就可以使代码不明显（很可能是错误的）。和&&。这里完全不清楚这是否是错误的。但是考虑到这些项目代码的整体质量，让我们假设在这里和其他几个地方，操作优先级存在错误：

V648“ &&”操作的优先级高于“ ||”的优先级操作。团队CPP 456
V648“ &&”操作的优先级高于“ ||”的优先级操作。显示器.CPP 1160
V648“ &&”操作的优先级高于“ ||”的优先级操作。DISPLAY.CPP 1571
V648“ &&”操作的优先级高于“ ||”的优先级操作。2594号房屋
V648“ &&”操作的优先级高于“ ||”的优先级操作。初始化CPP 2541

V617考虑检查状况。'|'的'（（（1L << STRUCT_CHRONOSPHERE））'自变量按位运算包含一个非零值。HOUSE.CPP 5089

typedef enum StructType : char {
  STRUCT_NONE=-1,
  STRUCT_ADVANCED_TECH,
  STRUCT_IRON_CURTAIN,
  STRUCT_WEAP,
  STRUCT_CHRONOSPHERE, // 3
  ....
}

#define  STRUCTF_CHRONOSPHERE (1L << STRUCT_CHRONOSPHERE)

UrgencyType HouseClass::Check_Build_Power(void) const
{
  ....
  if (State == STATE_THREATENED || State == STATE_ATTACKED) {
    if (BScan | (STRUCTF_CHRONOSPHERE)) {  // <=
      urgency = URGENCY_HIGH;
    }
  }
  ....
}

要检查变量中是否设置了某些位，请使用＆运算符，而不是|。由于这段代码中有错字，因此该条件始终为true。

V768枚举常量'WWKEY_RLS_BIT'用作布尔型变量。键盘.CPP 286

typedef enum {
  WWKEY_SHIFT_BIT = 0x100,
  WWKEY_CTRL_BIT  = 0x200,
  WWKEY_ALT_BIT   = 0x400,
  WWKEY_RLS_BIT   = 0x800,
  WWKEY_VK_BIT    = 0x1000,
  WWKEY_DBL_BIT   = 0x2000,
  WWKEY_BTN_BIT   = 0x8000,
} WWKey_Type;

int WWKeyboardClass::To_ASCII(int key)
{
  if ( key && WWKEY_RLS_BIT)
    return(KN_NONE);
  return(key);
}

我认为，在key参数中，他们想检查WWKEY_RLS_BIT掩码指定的某个位，但是打错了字。您应该使用按位＆运算符而不是&&来检查键码。

可疑格式

V523'then'语句等效于'else'语句。雷达CPP 1827

void RadarClass::Player_Names(bool on)
{
  IsPlayerNames = on;
  IsToRedraw = true;
  if (on) {
    Flag_To_Redraw(true);
//    Flag_To_Redraw(false);
  } else {
    Flag_To_Redraw(true);   // force drawing of the plate
  }
}

从前，开发人员对调试代码进行了评论。从那时起，代码一直是条件运算符，不同分支中的运算符相同。

完全相同的地方还有两个：

V523'then'语句等效于'else'语句。电池CPP 1792
V523'then'语句等效于'else'语句。雷达CPP 2274

V705可能忘记或注释了“其他”块，从而改变了程序的操作逻辑。NETDLG.CPP 1506

static int Net_Join_Dialog(void)
{
  ....
  /*...............................................................
  F4/SEND/'M' = edit a message
  ...............................................................*/
  if (Messages.Get_Edit_Buf()==NULL) {
    ....
  } else

  /*...............................................................
  If we're already editing a message and the user clicks on
  'Send', translate our input to a Return so Messages.Input() will
  work properly.
  ...............................................................*/
  if (input==(BUTTON_SEND | KN_BUTTON)) {
    input = KN_RETURN;
  }
  ....
}

由于评论过多，开发人员没有看到上面未定义的条件运算符。其余的else关键字的条件形成于else if构造之下，这很可能是对原始逻辑的更改。

V519为'ScoresPresent'变量连续分配了两次值。也许这是一个错误。检查行：539、541。INIT.CPP 541

bool Init_Game(int , char *[])
{
  ....
  ScoresPresent = false;
//if (CCFileClass("SCORES.MIX").Is_Available()) {
    ScoresPresent = true;
    if (!ScoreMix) {
      ScoreMix = new MixFileClass("SCORES.MIX");
      ThemeClass::Scan();
    }
//}

由于重构未完成而导致的另一个潜在缺陷。现在还不清楚ScoresPresent变量是否必须为true或仍然为false。

内存重新分配错误

V611使用“ new T []”运算符分配了内存，但使用“ delete”运算符释放了内存。考虑检查此代码。最好使用'delete [] poke_data;'。CCDDE.CPP 410

BOOL Send_Data_To_DDE_Server (char *data, int length, int packet_type)
{
  ....
  char *poke_data = new char [length + 2*sizeof(int)]; // <=
  ....
  if(DDE_Class->Poke_Server( .... ) == FALSE) {
    CCDebugString("C&C95 - POKE failed!\n");
    DDE_Class->Close_Poke_Connection();
    delete poke_data;                                  // <=
    return (FALSE);
  }

  DDE_Class->Close_Poke_Connection();

  delete poke_data;                                    // <=

  return (TRUE);
}

分析仪检测到一个错误，该错误与可以以不兼容的方式分配和释放内存的事实有关。要释放为数组分配的内存，您应该使用delete []运算符，而不是delete。

有几个这样的地方，它们都逐渐损害正在运行的应用程序（游戏）：

V611使用“ new T []”运算符分配了内存，但使用“ delete”运算符释放了内存。考虑检查此代码。最好使用'delete [] poke_data;'。CCDDE.CPP 416
V611 The memory was allocated using 'new T[]' operator but was released using the 'delete' operator. Consider inspecting this code. It's probably better to use 'delete [] temp_buffer;'. INIT.CPP 1302
V611 The memory was allocated using 'new T[]' operator but was released using the 'delete' operator. Consider inspecting this code. It's probably better to use 'delete [] progresspalette;'. MAPSEL.CPP 795
V611 The memory was allocated using 'new T[]' operator but was released using the 'delete' operator. Consider inspecting this code. It's probably better to use 'delete [] grey2palette;'. MAPSEL.CPP 796
V611使用“ new T []”运算符分配了内存，但使用“ delete”运算符释放了内存。考虑检查此代码。最好使用'delete [] poke_data;'。CCDDE.CPP 422
V611使用“ new T []”运算符分配了内存，但使用“ delete”运算符释放了内存。考虑检查此代码。最好使用'delete [] temp_buffer;'。初始化CPP 1139

V772调用void指针的“删除”运算符将导致未定义的行为。结局CPP 254

void GDI_Ending(void)
{
  ....
  void * localpal = Load_Alloc_Data(CCFileClass("SATSEL.PAL"));
  ....
  delete [] localpal;
  ....
}

delete和delete [] 运算符由于某种原因而分开。他们做了清除内存的另一项工作。当使用无类型指针时，编译器不知道指针指向的数据类型。在C ++语言标准中，编译器的行为未定义。

还发现了许多此类分析器警告：

V772为空指针调用“删除”运算符将导致未定义的行为。HEAP.CPP 284
V772调用void指针的“删除”运算符将导致未定义的行为。初始化CPP 728
V772调用void指针的“删除”运算符将导致未定义的行为。第134章
V772调用void指针的“删除”运算符将导致未定义的行为。MIXFILE.CPP 391
V772 Calling a 'delete' operator for a void pointer will cause undefined behavior. MSGBOX.CPP 423
V772 Calling a 'delete' operator for a void pointer will cause undefined behavior. SOUNDDLG.CPP 407
V772 Calling a 'delete' operator for a void pointer will cause undefined behavior. BUFFER.CPP 126
V772 Calling a 'delete' operator for a void pointer will cause undefined behavior. BUFF.CPP 162
V772 Calling a 'delete' operator for a void pointer will cause undefined behavior. BUFF.CPP 212
V772 Calling a 'delete' operator for a void pointer will cause undefined behavior. BFIOFILE.CPP 330
V772 Calling a 'delete' operator for a void pointer will cause undefined behavior. EVENT.CPP 934
V772 Calling a 'delete' operator for a void pointer will cause undefined behavior. HEAP.CPP 318
V772 Calling a 'delete' operator for a void pointer will cause undefined behavior. INIT.CPP 3851
V772 Calling a 'delete' operator for a void pointer will cause undefined behavior. MIXFILE.CPP 130
V772 Calling a 'delete' operator for a void pointer will cause undefined behavior. MIXFILE.CPP 430
V772 Calling a 'delete' operator for a void pointer will cause undefined behavior. MIXFILE.CPP 447
V772 Calling a 'delete' operator for a void pointer will cause undefined behavior. MIXFILE.CPP 481
V772 Calling a 'delete' operator for a void pointer will cause undefined behavior. MSGBOX.CPP 461
V772 Calling a 'delete' operator for a void pointer will cause undefined behavior. QUEUE.CPP 2982
V772 Calling a 'delete' operator for a void pointer will cause undefined behavior. QUEUE.CPP 3167
V772调用void指针的“删除”运算符将导致未定义的行为。声音DLG.CPP 406

V773在不释放“ progresspalette”指针的情况下退出了该功能。可能发生内存泄漏。MAPSEL.CPP 258

void Map_Selection(void)
{
  ....
  unsigned char *grey2palette    = new unsigned char[768];
  unsigned char *progresspalette = new unsigned char[768];
  ....
  scenario = Scenario + ((house == HOUSE_GOOD) ? 0 : 14);
  if (house == HOUSE_GOOD) {
    lastscenario = (Scenario == 14);
    if (Scenario == 15) return;
  } else {
    lastscenario = (Scenario == 12);
    if (Scenario == 13) return;
  }
  ....
}

“如果您根本不释放内存，那么选择操作员绝对不会出错！” -也许程序员认为。

但是随后会发生内存泄漏，这也是一个错误。在函数末尾的某个位置释放了内存，但是在此之前，有很多地方发生了从函数有条件退出的情况，并且指针grey2palette和progresspalett的内存没有释放。

杂项

V570为其分配了“ hdr-> MagicNumber”变量。邮编806

struct CommHdr {
  unsigned short MagicNumber;
  unsigned char Code;
  unsigned long PacketID;
} *hdr;

void CommBufferClass::Mono_Debug_Print(int refresh)
{
  ....
  hdr = (CommHdr *)SendQueue[i].Buffer;
  hdr->MagicNumber = hdr->MagicNumber;
  hdr->Code = hdr->Code;
  ....
}

CommHdr结构的两个字段用它们自己的值初始化。我认为这是没有意义的操作，但是它执行了很多次：

V570为其分配了“ hdr->代码”变量。国会大厦CPP 807
V570为其分配了“ hdr-> MagicNumber”变量。第931章
V570为其分配了“ hdr->代码”变量。国会大厦CPP 932
V570为其分配了“ hdr-> MagicNumber”变量。联合会987
V570为其分配了“ hdr->代码”变量。第988章
V570将'obj'变量分配给它自己。MAP.CPP 1132
V570为其分配了“ hdr-> MagicNumber”变量。国会大厦CPP 910
V570为其分配了“ hdr->代码”变量。坎伯CPP 911
V570为其分配了“ hdr-> MagicNumber”变量。第1040章
V570为其分配了“ hdr->代码”变量。国会大厦CPP 1041
V570为其分配了“ hdr-> MagicNumber”变量。第1104章
V570为其分配了“ hdr->代码”变量。第1105章
V570将'obj'变量分配给它自己。MAP.CPP 1279

V591非无效函数应返回一个值。HEAP.H 123

int FixedHeapClass::Free(void * pointer);

template<class T>
class TFixedHeapClass : public FixedHeapClass
{
  ....
  virtual int Free(T * pointer) {FixedHeapClass::Free(pointer);};
};

Free类的函数TFixedHeapClass no运算符的return语句。最有趣的是，被调用的FixedFiapHeapClass :: Free函数具有int类型的返回值。程序员很可能只是忘了编写return语句，而现在该函数返回了一个无法理解的值。

V672可能不需要在此处创建新的“损坏”变量。函数的参数之一具有相同的名称，并且该参数是引用。检查行：1219、1278。BUILDING.CPP 1278

ResultType BuildingClass::Take_Damage(int & damage, ....)
{
  ....
  if (tech && tech->IsActive && ....) {
    int damage = 500;
    tech->Take_Damage(damage, 0, WARHEAD_AP, source, forced);
  }
  ....
}

损坏参数通过引用传递。因此，功能体有望更改此变量的值。但是在一处，开发人员声明了一个具有相同名称的变量。因此，值500将存储在局部变量损坏中，而不是函数参数中。也许打算采取不同的行为。

另一个这样的地方：

V672可能不需要在此处创建新的“损坏”变量。函数的参数之一具有相同的名称，并且该参数是引用。检查线：4031、4068。TECHNO.CPP 4068

V762虚函数可能被错误地覆盖。请参见派生类“ BulletClass”和基类“ ObjectClass”中函数“ Occupy_List”的第一个参数。子弹头90

class ObjectClass : public AbstractClass
{
  ....
  virtual short const * Occupy_List(bool placement=false) const; // <=
  virtual short const * Overlap_List(void) const;
  ....
};

class BulletClass : public ObjectClass,
                    public FlyClass,
                    public FuseClass
{
  ....
  virtual short const * Occupy_List(void) const;                 // <=
  virtual short const * Overlap_List(void) const {return Occupy_List();};
  ....
};

分析器在覆盖Occupy_List虚拟函数时检测到潜在的错误。这可能导致在运行时调用错误的函数。

其他一些可疑的地方：

V762虚函数可能被错误地覆盖。请参见派生类“ TurretClass”和基类“ DriveClass”中函数“ Ok_To_Move”的限定符。TURRET.H 76
V762虚函数可能被错误地覆盖。请参见派生类“ HelpClass”和基类“ DisplayClass”中函数“ Help_Text”的第四个参数。帮助55
V762虚函数可能被错误地覆盖。请参见派生类“ MapEditClass”和基类“ HelpClass”中函数“ Draw_It”的第一个参数。MAPEDIT.H 187
V762 It is possible a virtual function was overridden incorrectly. See first argument of function 'Occupy_List' in derived class 'AnimClass' and base class 'ObjectClass'. ANIM.H 80
V762 It is possible a virtual function was overridden incorrectly. See first argument of function 'Overlap_List' in derived class 'BulletClass' and base class 'ObjectClass'. BULLET.H 102
V762 It is possible a virtual function was overridden incorrectly. See qualifiers of function 'Remap_Table' in derived class 'BuildingClass' and base class 'TechnoClass'. BUILDING.H 281
V762 It is possible a virtual function was overridden incorrectly. See fourth argument of function 'Help_Text' in derived class 'HelpClass' and base class 'DisplayClass'. HELP.H 58
V762虚函数可能被错误地覆盖。请参见派生类“ AnimClass”和基类“ ObjectClass”中函数“ Overlap_List”的第一个参数。动漫90

V763参数“ coord”在使用前总是在功能体中重写。DISPLAY.CPP 4031

void DisplayClass::Set_Tactical_Position(COORDINATE coord)
{
  int xx = 0;
  int yy = 0;

  Confine_Rect(&xx, &yy, TacLeptonWidth, TacLeptonHeight,
    Cell_To_Lepton(MapCellWidth) + GlyphXClientSidebarWidthInLeptons,
    Cell_To_Lepton(MapCellHeight));

  coord = XY_Coord(xx + Cell_To_Lepton(MapCellX), yy + Cell_To_Lepton(....));

  if (ScenarioInit) {
    TacticalCoord = coord;
  }
  DesiredTacticalCoord = coord;
  IsToRedraw = true;
  Flag_To_Redraw(false);
}

coord 参数立即在函数主体中被覆盖。未使用旧值。当一个函数有参数并且不依赖于参数时，这是非常可疑的。然后有一些坐标。

这个地方值得一看：

V763参数“ coord”在使用前总是在功能体中重写。DISPLAY.CPP 4251

V507指向本地数组'localpalette'的指针存储在此数组的范围之外。这样的指针将变为无效。MAPSEL.CPP 757

extern "C" unsigned char *InterpolationPalette;

void Map_Selection(void)
{
  unsigned char localpalette[768];
  ....
  InterpolationPalette = localpalette;
  ....
}

游戏代码中有很多全局变量。在那时，这可能是一种常见的编码方法。但是现在它被认为是坏的，甚至是危险的。

本地数组localpalette存储在InterpolationPalette指针中，在退出函数后它将变得无效。

几个更危险的地方：

V507指向本地数组'localpalette'的指针存储在此数组的范围之外。这样的指针将变为无效。MAPSEL.CPP 769
V507指向本地数组“缓冲区”的指针存储在此数组的范围之外。这样的指针将变为无效。WINDOWS.CPP 458

结论

正如我在第一份报告中所写，我们希望电子艺术的新项目质量更高。通常，游戏开发商会积极购买PVS-Studio。现在，游戏的预算非常大，因此没有人需要修复生产中的错误的额外费用。而且，在编码的早期阶段修复错误几乎不需要时间和其他资源。

我们邀请您下载并尝试在我们网站上的所有项目上使用PVS-Studio。

如果您想与说英语的人分享这篇文章，请使用翻译链接：Svyatoslav Razmyslov。命令与征服游戏代码：90年代的错误。第二卷。

命令与征服游戏代码：90年代的错误。第二卷

介绍

条件错误

可疑格式

内存重新分配错误

杂项

结论

More articles: