零,一,二,弗雷迪会接你

图片1

这是一系列文章的延续,这些文章的标题为“程序员的恐怖”。这次,我们将讨论与数字0、1、2的使用相关的典型错字模式。无论您是用C,C ++,C#还是Java编写都没有关系。如果您使用常数0、1、2或这些数字包含在变量名称中,那么很有可能Freddy将在晚上拜访您。阅读并且以后不要说您没有受到警告。


介绍


我将继续发表一系列文章,探讨人们如何犯错的典型模式。以前的出版物:
  1. 最后一行效果
  2. C / C ++世界上最危险的功能
  3. 邪恶生活在比较功能中

这次不是我注意到了这种模式,而是我的同事Svyatoslav Razmyslov注意到了。他提请注意以下事实:他经常在文章中描述其名称中出现包含数字1和2的变量的问题,斯维亚托斯拉夫(Svyatoslav)建议我更详细地研究该主题,事实证明它非常富有成果。事实证明,由于人们对索引0、1、2或包含此类数字的变量名称感到困惑,我们的错误集合包含大量不正确的代码片段。揭示了一个有趣的新规律,将在下面讨论。我感谢Svyatoslav迅速调查此主题并将本文献给他。

图14

Svyatoslav Razmyslov,经理,细心的bug 追捕者 ,只是一个有才华的人。

本文的目的是什么?展示我们所有人容易犯错误和错别字。如果警告程序员,他们将在代码审查过程中更加专心,专注于命运不佳的0、1、2。此外,程序员将能够更好地感受到有助于注意到此类错误的静态代码分析器的价值。这与PVS-Studio广告无关(尽管也是如此:)。到目前为止,许多程序员发现静态分析是多余的,他们宁愿专注于自己的准确性和代码审查。不幸的是,尝试编写没有错误的代码是好的,但还不够。本文将再次清楚地说明这一点。

没有人能避免这种错误。在下面,即使在Qt,Clang,Hive,LibreOffice,Linux Kernel,.NET Compiler Platform,XNU内核,Mozilla Firefox等知名项目中,您也会看到史诗般的炸弹。这些不是一些罕见的罕见错误,而是最常见的错误。不服气吗?然后开始吧!

“ Ch不值钱!告诉我这些错误!”

©Linus Torvalds修改的报价。

索引数组时常量中的错别字


通常,在我们的文章中,我们会根据发现的错误给出警告。这次我将忽略这些警告,因为没有它们,错误将很容易被察觉和理解。但是,尽管这些错误在一小段代码中立即可见,但它们知道如何隐藏在项目代码中。

当与用于索引数组的数字文字混淆时,让我们从错误开始。尽管这些错误很平庸,但其中有许多错误并没有在学生的实验室工作中显示出来。

XNU内核项目,C语言
uint32_t
gss_krb5_3des_unwrap_mbuf(....)
{
  ....
  for (cflag = 1; cflag >= 0; cflag--) {
    *minor = gss_krb5_3des_token_get(
       ctx, &itoken, wrap, &hash, &offset, &length, reverse);
    if (*minor == 0)
      break;
    wrap.Seal_Alg[0] = 0xff;
    wrap.Seal_Alg[0] = 0xff;
  }
  ....
}

该行已复制,但忘记更正索引。据我了解,它应该写在这里:
wrap.Seal_Alg[0] = 0xff;
wrap.Seal_Alg[1] = 0xff;

LibreOffice Project,C ++
Sequence< OUString > FirebirdDriver::
  getSupportedServiceNames_Static() throw (RuntimeException)
{
  Sequence< OUString > aSNS( 2 );
  aSNS[0] = "com.sun.star.sdbc.Driver";
  aSNS[0] = "com.sun.star.sdbcx.Driver";
  return aSNS;
}

与前面的情况一样,该行被复制,但是忘记用1来修复0。只有字符串文字才被更正。

有人可能会提出一个哲学问题,如何在四线功能中犯这样的错误?一切皆有可能。这是编程。

Quake-III-Arena项目,C语言
int VL_FindAdjacentSurface(....)
{
  ....
  if (fabs(dir[0]) > test->radius ||
      fabs(dir[1]) > test->radius ||
      fabs(dir[1]) > test->radius)
  {
  ....
}

在复制的行中,他们忘记了dir [2]替换dir [1 ]结果,沿Z轴的值不受控制。

OpenCOLLADA项目,C ++
struct short2
{
  short values[2];
  short2(short s1, short s2)
  {
    values[0] = s1;
    values[2] = s2;
  }
  ....
};

是的,即使在这么短的构造函数中,您也可以在初始化数组时设法超越数组的边界。

图8


Godot引擎项目,C ++
Array PhysicsDirectSpaceState::_cast_motion(....)
{
  ....
  Array ret(true);
  ret.resize(2);
  ret[0]=closest_safe;
  ret[0]=closest_unsafe;
  return ret;
}

无需评论。

星号项目,C语言
static void sip_threadinfo_destructor(void *obj)
{
  struct sip_threadinfo *th = obj;
  struct tcptls_packet *packet;

  if (th->alert_pipe[1] > -1) {            // <=
    close(th->alert_pipe[0]);
  }
  if (th->alert_pipe[1] > -1) {
    close(th->alert_pipe[1]);
  }
  th->alert_pipe[0] = th->alert_pipe[1] = -1;
  ....
}

通常,编写相同类型的块时,错误位于底层块中。在此之前,所有考虑的案例都只是这样。这里的错字是在一个不寻常的地方,即在第一块。为什么会发生很难说。我别无选择,只能带来一张独角兽耸耸肩的照片:

图9


打开CASCADE技术项目,C ++
inline void Prepend(const Standard_Integer theIndex)
{
  if (myIndex[1] >= 0)
    Standard_OutOfRange::Raise ("BRepMesh_PairOfIndex....");

  myIndex[1] = myIndex[0];
  myIndex[1] = theIndex;
}

在数组的同一单元格中两次复制不同的值。一个明显的错误,但由于我对项目代码不熟悉,如何解决该问题对我来说并不明确。因此,在我们的团队向他们指出此错误之后,我只是研究了开发人员如何纠正代码。正确的选项:
myIndex[1] = myIndex[0];
myIndex[0] = theIndex;

跨蛋白质组学管道项目,C ++
void ASAPRatio_getProDataStrct(proDataStrct *data,
                               char **pepBofFiles)
{
  ....
  if (data->indx == -1) {
    data->ratio[0] = -2.;
    data->ratio[0] = 0.;             // <=
    data->inv_ratio[0] = -2.;
    data->inv_ratio[1] = 0.;
    return;
  }
  ....
}

我担心研究包中会包含此类错误。跨蛋白质组学管道旨在解决生物学领域的问题。这可以决定并“调查”。这个软件包通常会发现很多有趣的东西:2012检查,2013 检查也许您可以再次尝试查看该项目。

ITK项目,C ++语言

我们面临着另一个用于进行医学领域研究的项目:Medicine Insight细分和注册工具包(ITK)。项目是不同的,但是错误是相同的。
template< typename TCoordRepType >
void
VoronoiDiagram2D< TCoordRepType >::SetOrigin(PointType vorsize)
{
  m_VoronoiBoundaryOrigin[0] = vorsize[0];
  m_VoronoiBoundaryOrigin[0] = vorsize[1];
}

ITK项目,C ++


int itkPointSetToSpatialObjectDemonsRegistrationTest(....)
{
  ....
  // Set its position
  EllipseType::TransformType::OffsetType offset;
  offset[0]=50;
  offset[1]=50;
  offset[1]=50;
  ....
}

清洁复制粘贴。

ReactOS项目,C ++
HPALETTE CardWindow::CreateCardPalette()
{
  ....
  //include button text colours
  cols[0] = RGB(0, 0, 0);
  cols[1] = RGB(255, 255, 255);

  //include the base background colour
  cols[1] = crBackgnd;

  //include the standard button colours...
  cols[3] = CardButton::GetHighlight(crBackgnd);
  cols[4] = CardButton::GetShadow(crBackgnd);
  cols[5] = CardButton::GetFace(crBackgnd);
  ....
}

最有可能的是,应将crBackgnd常量写入cols单元格[2]

Coin3D项目,C ++
SoVRMLInline::GLRender(SoGLRenderAction * action)
{
  ....
  if ((size[0] >= 0.0f && size[1] >= 0.0f && size[1] >= 0.0f) &&
      ((vis == ALWAYS) ||
       (vis == UNTIL_LOADED && child == NULL))) {
  ....
}

size [1] 数组的元素被仔细检查,并且大小[2]的元素未选中。这就是奇怪的伪像出现在图像上的方式。

OpenCV项目,C ++
bool Jpeg2KDecoder::readHeader()
{
  ....
  cmptlut[0] = ....
  cmptlut[1] = ....
  cmptlut[2] = ....
  if( cmptlut[0] < 0 || cmptlut[1] < 0 || cmptlut[0] < 0 )
    result = false;
  ....
}

直接感觉到表达式cmptlut [0] <0通过复制被复制了两次,但仅在一个位置纠正了零。

可视化工具包(VTK)项目,C ++
void vtkImageStencilRaster::PrepareForNewData(....)
{
  ....
  if (allocateExtent &&
      allocateExtent[1] >= allocateExtent[1])
  ....
}

在下文中,我将不评论许多此类错误。有什么要评论的?查看此类代码片段时,最主要的感觉是,尽管错误很简单,但这并不意味着程序员会注意到它。

可视化工具包(VTK)项目,C ++
template <class iterT>
void vtkDataSetAttributesCopyValues(....)
{
  ....
  inZPtr +=
    (outExt[0] - outExt[0])*inIncs[0] * data_type_size +
    (outExt[2] - outExt[2])*inIncs[1] * data_type_size +
    (outExt[4] - outExt[4])*inIncs[2] * data_type_size;
  ....
}

在这里,程序员显然急于更快地编写代码。用另一种方式很难解释他是怎么犯了三次错误。从数组中减去元素。结果是此代码是等效的:
inZPtr +=
  (0)*inIncs[0] * data_type_size +
  (0)*inIncs[1] * data_type_size +
  (0)*inIncs[2] * data_type_size;

但是,可以进一步减少此代码:
inZPtr += 0;

奢侈地 在代码中有一个很长的严肃表情,实际上什么也没做。我喜欢这种情况。

可视化工具包(VTK)项目,C ++语言

仓促编写代码的类似情况。
void vtkPiecewiseControlPointsItem::SetControlPoint(
  vtkIdType index, double* newPos)
{
  double oldPos[4];
  this->PiecewiseFunction->GetNodeValue(index, oldPos);
  if (newPos[0] != oldPos[0] || newPos[1] != oldPos[1] ||
      newPos[2] != oldPos[2] || newPos[2] != oldPos[2])
    {
      this->PiecewiseFunction->SetNodeValue(index, newPos);
    }
}

比较newPos [2]!= OldPos [2]重复两次

自适应通信环境(ACE)项目,C ++
bool URL_Base::strip_scheme (ACE_CString& url_string)
{
  ....
  ACE_CString::size_type pos = url_string.find (':');
  if (pos > 0 &&
      url_string[pos+1] == '/' &&
      url_string[pos+1] == '/')
  {
    ....
    // skip '<protocol>://'
    url_string = url_string.substr (pos+3);
  }
  ....
}

条件必须验证在冒号之后遇到两个斜杠。换句话说,搜索子字符串“://”。由于有错字,支票是“盲目的”,可以将任何字符算作第二个斜线。

IPP样本项目,C ++
void MeBase::MakeVlcTableDecision()
{
  ....
  Ipp32s BestMV =
    IPP_MIN(IPP_MIN(m_cur.MvRate[0],m_cur.MvRate[1]),
                    IPP_MIN(m_cur.MvRate[2],m_cur.MvRate[3]));
  Ipp32s BestAC =
    IPP_MIN(IPP_MIN(m_cur.AcRate[0],m_cur.AcRate[1]),
                    IPP_MIN(m_cur.AcRate[2],m_cur.AcRate[2]));
  ....
}

在传递给宏的参数中有一个错字:
IPP_MIN(m_cur.AcRate[2],m_cur.AcRate[2])

事实证明,至少选择了两个相同的值。实际上,应该这样写:
IPP_MIN(m_cur.AcRate[2],m_cur.AcRate[3])

顺便说一句,这段代码可以证明标准库的有用性。如果您这样写:
Ipp32s BestMV = std::min_element(begin(m_cur.MvRate), end(m_cur.MvRate));
Ipp32s BestAC = std::min_element(begin(m_cur.AcRate), end(m_cur.AcRate));

该代码将变得更短,更不容易出错。实际上,相同类型的代码越少,正确编写代码的可能性就越大。

Project Audacity,C ++
sampleCount VoiceKey::OnBackward (....) {
  ....
  int atrend = sgn(buffer[samplesleft - 2]-
                   buffer[samplesleft - 1]);
  int ztrend = sgn(buffer[samplesleft - WindowSizeInt-2]-
                   buffer[samplesleft - WindowSizeInt-2]);
  ....
}

正确的表达式是:
int ztrend = sgn(buffer[samplesleft - WindowSizeInt-2]-
                 buffer[samplesleft - WindowSizeInt-1]);

PDFium项目,C ++语言
void sycc420_to_rgb(opj_image_t* img) {
  ....
  opj_image_data_free(img->comps[0].data);
  opj_image_data_free(img->comps[1].data);
  opj_image_data_free(img->comps[2].data);
  img->comps[0].data = d0;
  img->comps[1].data = d1;
  img->comps[2].data = d2;
  img->comps[1].w = yw;                 // 1
  img->comps[1].h = yh;                 // 1
  img->comps[2].w = yw;                 // 1
  img->comps[2].h = yh;                 // 1
  img->comps[1].w = yw;                 // 2
  img->comps[1].h = yh;                 // 2
  img->comps[2].w = yw;                 // 2
  img->comps[2].h = yh;                 // 2
  img->comps[1].dx = img->comps[0].dx;
  img->comps[2].dx = img->comps[0].dx;
  img->comps[1].dy = img->comps[0].dy;
  img->comps[2].dy = img->comps[0].dy;
}

复制了初始化结构的一系列操作。那些带有注释// 2的行可以删除,并且什么都不会改变。我怀疑是否在文章中包含这段代码。这不是一个错误,索引也不是一个错误。但是,这些额外的代码很可能是由于程序员在所有这些类成员和索引1、2中感到困惑而出现的。因此,我认为这段代码非常适合演示数字混淆的难易程度。

CMake项目,C

下面讨论的代码不是CMake开发人员编写的,而是借用的。根据文件开头的注释判断,utf8_encode函数它是由Tim Kientzle于2007年编写的。此后,此功能一直在项目之间徘徊,其中有很多功能。我没有研究原始来源的问题,因为这不是重点。由于此代码在CMake项目中,因此该错误适用于CMake。
static char *
utf8_encode(const wchar_t *wval)
{
  ....
  p[0] = 0xfc | ((wc >> 30) & 0x01);
  p[1] = 0x80 | ((wc >> 24) & 0x3f);
  p[1] = 0x80 | ((wc >> 18) & 0x3f);
  p[2] = 0x80 | ((wc >> 12) & 0x3f);
  p[3] = 0x80 | ((wc >> 6) & 0x3f);
  p[4] = 0x80 | (wc & 0x3f);
  p += 6;
  ....
}

如您所见,索引有些混乱。数组元素p [1]中有两次记录如果您研究附近的代码,则很明显,正确的代码应如下所示:
p[0] = 0xfc | ((wc >> 30) & 0x01);
p[1] = 0x80 | ((wc >> 24) & 0x3f);
p[2] = 0x80 | ((wc >> 18) & 0x3f);
p[3] = 0x80 | ((wc >> 12) & 0x3f);
p[4] = 0x80 | ((wc >> 6) & 0x3f);
p[5] = 0x80 | (wc & 0x3f);
p += 6;

注意

请注意,本章中讨论的所有错误均与C或C ++代码有关。没有C#或Java代码!

这很有趣,我没想到这一点。我认为,所考虑的错别字不取决于语言。并且在接下来的章节中,其他语言的代码中的错误确实会出现。我认为这只是一个巧合。与C / C ++相比,PVS-Studio分析仪更晚开始支持C#/ Java语言,我们只是没有设法在数据库中累积相应的错误示例。

但是,观察仍然很有趣。显然,C和C ++程序员在处理数组时喜欢使用数字0、1和2 :)。

名称拼写错误


这将是最大的部分。人们很容易对a1a2之类的名称感到困惑您似乎在这里会感到困惑?能够。简单。现在,读者将能够验证这一点。

蜂巢项目,Java
@Override
public List<ServiceInstance> getAllInstancesOrdered() {
  List<ServiceInstance> list = new LinkedList<>();
  list.addAll(instances.values());
  Collections.sort(list, new Comparator<ServiceInstance>() {
    @Override
    public int compare(ServiceInstance o1, ServiceInstance o2) {
      return o2.getWorkerIdentity().compareTo(o2.getWorkerIdentity());
    }
  });
  return list;
}

比较函数compare需要两个对象:o1o2但是由于打字错误,仅进一步使用了o2

有趣的是,由于复制粘贴,此错误迁移到另一个函数:
@Override
public List<ServiceInstance> getAllInstancesOrdered() {
  List<ServiceInstance> list = new LinkedList<>();
  readLock.lock();
  try {
    list.addAll(instances.values());
  } finally {
    readLock.unlock();
  }
  Collections.sort(list, new Comparator<ServiceInstance>() {
    @Override
    public int compare(ServiceInstance o1, ServiceInstance o2) {
      return o2.getWorkerIdentity().compareTo(o2.getWorkerIdentity());
    }
  });
  return list;
}

图10


Infer.NET项目,C#语言
private void MergeParallelTransitions()
{
  ....
  if (double.IsInfinity(transition1.Weight.Value) &&    
      double.IsInfinity(transition1.Weight.Value))
  ....
}

《毁灭战士3》项目,C ++
uint AltOp::fixedLength()
{
  uint l1 = exp1->fixedLength();
  uint l2 = exp1->fixedLength();

  if (l1 != l2 || l1 == ~0u)
    return ~0;

  return l1;
}

如果有人没有立即注意到错字,那么您需要查看l2变量被初始化的那一行应该使用exp2

Source Engine SDK项目,C ++
void GetFPSColor( int nFps, unsigned char ucColor[3] )
{
  ....
  int nFPSThreshold1 = 20;
  int nFPSThreshold2 = 15;

  if (IsPC() &&
      g_pMaterialSystemHardwareConfig->GetDXSupportLevel() >= 95)
  {
    nFPSThreshold1 = 60;
    nFPSThreshold1 = 50;
  }
  ....
}

正确地:
nFPSThreshold1 = 60;
nFPSThreshold2 = 50;

Linux内核项目,C语言

顺便说一句,错别字不仅可以使用变量名,还可以使用宏名。现在将有几个这样的例子。
int private_ioctl(struct vnt_private *pDevice, struct ifreq *rq)
{
  ....
  if (sStartAPCmd.byBasicRate & BIT3) {
    pMgmt->abyIBSSSuppRates[2] |= BIT7;
    pMgmt->abyIBSSSuppRates[3] |= BIT7;
    pMgmt->abyIBSSSuppRates[4] |= BIT7;
    pMgmt->abyIBSSSuppRates[5] |= BIT7;
  } else if (sStartAPCmd.byBasicRate & BIT2) {
    pMgmt->abyIBSSSuppRates[2] |= BIT7;
    pMgmt->abyIBSSSuppRates[3] |= BIT7;
    pMgmt->abyIBSSSuppRates[4] |= BIT7;
  } else if (sStartAPCmd.byBasicRate & BIT1) {  // <=
    pMgmt->abyIBSSSuppRates[2] |= BIT7;
    pMgmt->abyIBSSSuppRates[3] |= BIT7;
  } else if (sStartAPCmd.byBasicRate & BIT1) {  // <=
    pMgmt->abyIBSSSuppRates[2] |= BIT7;
  } else {
    /* default 1,2M */
    pMgmt->abyIBSSSuppRates[2] |= BIT7;
    pMgmt->abyIBSSSuppRates[3] |= BIT7;
  }
  ....
}

如您所见,两次使用了一个称为BIT1的掩码,这使第二次检查毫无意义。第二条注释条件语句的正文将永远不会执行。

CMaNGOS项目,C ++
void AttackedBy(Unit* pAttacker) override
{
  ....
  DoScriptText(urand(0, 1) ?
               SAY_BELNISTRASZ_AGGRO_1 :
               SAY_BELNISTRASZ_AGGRO_1,
               m_creature, pAttacker);
  ....
}

在游戏中计划了随机行为,但始终选择相同的常量SAY_BELNISTRASZ_AGGRO_1

Vangers项目:C之路一
const char* iGetJoyBtnNameText(int vkey,int lang)
{
  ....
  if (vkey >= VK_STICK_SWITCH_1 && vkey <= VK_STICK_SWITCH_9)
  {
     ret = (lang)
      ? iJoystickStickSwitch2[vkey - VK_STICK_SWITCH_1]
      : iJoystickStickSwitch2[vkey - VK_STICK_SWITCH_1];
    return ret;
  }
  ....
}

根据旁边的代码判断,正确的选项应如下所示:
ret = (lang)
  ? iJoystickStickSwitch2[vkey - VK_STICK_SWITCH_1]
  : iJoystickStickSwitch1[vkey - VK_STICK_SWITCH_1];

RT线程项目,C语言
uint8_t can_receive_message_length(uint32_t can_periph,
                                   uint8_t fifo_number)
{
  uint8_t val = 0U;

  if(CAN_FIFO0 == fifo_number){
    val = (uint8_t)(CAN_RFIFO0(can_periph) & CAN_RFIFO_RFL0_MASK);
  }else if(CAN_FIFO0 == fifo_number){
    val = (uint8_t)(CAN_RFIFO1(can_periph) & CAN_RFIFO_RFL0_MASK);
  }else{
    /* illegal parameter */
  }
  return val;
}

RT-Thread是嵌入式设备的开源实时操作系统。在这里,我们看到FIFO 0和FIFO 1之间的混淆。在某处,有人会遇到有故障的设备。

图11


错误在这里:
if      (CAN_FIFO0 == fifo_number){
....
}else if(CAN_FIFO0 == fifo_number){

第二张支票始终为假。正确地:
if      (CAN_FIFO0 == fifo_number){
....
}else if(CAN_FIFO1 == fifo_number){

蜂巢项目,Java
private void
generateDateTimeArithmeticIntervalYearMonth(String[] tdesc) throws Exception {
  String operatorName = tdesc[1];
  String operatorSymbol = tdesc[2];
  String operandType1 = tdesc[3];
  String colOrScalar1 = tdesc[4];
  String operandType2 = tdesc[5];
  String colOrScalar2 = tdesc[6];
  ....
  if (colOrScalar1.equals("Col") && colOrScalar1.equals("Column")) {
    ....
  } else if (colOrScalar1.equals("Col") && colOrScalar1.equals("Scalar")) {
    ....
}

PVS-Studio分析仪会立即显示2个错误:
  1. 存储在colOrScalar1中的字符串不能同时等于Col和Column字符串。
  2. 存储在colOrScalar1中的字符串不能同时等于Col和Scalar字符串。

变量名显然很混乱。

Shareaza项目,C ++语言
void CDownloadWithSources::MergeMetadata(const CXMLElement* pXML)
{
  CQuickLock pLock( Transfers.m_pSection );

  CXMLAttribute* pAttr1 =
    m_pXML->GetAttribute(CXMLAttribute::schemaName);
  CXMLAttribute* pAttr2 =
    pXML->GetAttribute(CXMLAttribute::schemaName);

  if (pAttr1 && pAttr2 &&
      !pAttr1->GetValue().CompareNoCase(pAttr1->GetValue()))
    ....
}

正确地:
pAttr1->GetValue().CompareNoCase(pAttr2->GetValue())

注意

让我们稍作停顿。有人担心,通过一堆平庸的错误,我们会忘记为什么要这样做。

任务是不要嘲笑别人的代码。所有这些都不是戳手指说“哈哈,好吧,你必须这么做”的理由。想这个理由!我们团队

的出版物旨在表明,我们每个人都不免犯错误。本文中描述的错误在代码中的出现频率比您预期的要高得多。同样重要的是,丢失0、1、2的可能性几乎与程序员的资格无关。

认识到人们容易犯错误是很有用的。否则,您将无法采取下一步措施来提高代码的质量和可靠性。人们知道我们都错了,所以人们开始尝试在最早的阶段使用编码标准,代码审查,单元测试,静态和动态分析器来识别错误。这很棒。

为什么事情这么容易理解?不幸的是,在与大量开发人员进行交流时,我们不得不声明并非所有人都清楚这一点。许多人的自尊心太高,他们根本不认为自己有能力犯简单的错误。这是可悲的。

如果您是团队负责人/经理,那么我邀请您同时熟悉本说明

Qt项目,C ++
AtomicComparator::ComparisonResult
IntegerComparator::compare(const Item &o1,
                           const AtomicComparator::Operator,
                           const Item &o2) const
{
  const Numeric *const num1 = o1.as<Numeric>();
  const Numeric *const num2 = o1.as<Numeric>();

  if(num1->isSigned() || num2->isSigned())
  ....
}

正确地:
const Numeric *const num2 = o2.as<Numeric>();

Android项目,C ++语言
static inline bool isAudioPlaybackRateEqual(
  const AudioPlaybackRate &pr1,
  const AudioPlaybackRate &pr2)
{
    return fabs(pr1.mSpeed - pr2.mSpeed) <
             AUDIO_TIMESTRETCH_SPEED_MIN_DELTA &&
           fabs(pr1.mPitch - pr2.mPitch) <
             AUDIO_TIMESTRETCH_PITCH_MIN_DELTA &&
           pr2.mStretchMode == pr2.mStretchMode &&
           pr2.mFallbackMode == pr2.mFallbackMode;
}

一次输入两个错字,因此将变量pr2.mStretchModepr2.mFallbackMode与自己进行比较。

Boost项目,C ++
point3D operator/(const point3D &p1, const point3D &p2)
{
  return point3D(p1.x/p2.x, p1.y/p2.y, p1.z/p1.z);
}

最后,他们密封并划分了变量p1.z本身。

Clang专案,C ++
bool haveSameType(QualType Ty1, QualType Ty2) {
  return (Context.getCanonicalType(Ty1) ==
          Context.getCanonicalType(Ty2) ||
          (Ty2->isIntegerType() &&
           Ty2->isIntegerType()));
}

是的,PVS-Studio分析仪在编译器中发现了类似的错误。正确地:
(Ty1->isIntegerType() &&
 Ty2->isIntegerType())

Clang专案,C ++
Instruction *InstCombiner::visitXor(BinaryOperator &I) {
  ....
  if (Op0I && Op1I && Op0I->isShift() &&
      Op0I->getOpcode() == Op1I->getOpcode() &&
      Op0I->getOperand(1) == Op1I->getOperand(1) &&
      (Op1I->hasOneUse() || Op1I->hasOneUse())) {
  ....
}

正确地:
(Op0I->hasOneUse() || Op1I->hasOneUse())

Qt项目,C ++
inline bool qCompare(QImage const &t1, QImage const &t2, ....)
{
  ....
  if (t1.width() != t2.width() || t2.height() != t2.height()) {
  ....
}

NCBI基因组工作台项目,C ++
static bool s_PCRPrimerSetLess(const CPCRPrimerSet& s1, const CPCRPrimerSet& s2)
{
  if (!s1.IsSet() && s1.IsSet()) {
    return true;
  } else if (s1.IsSet() && !s2.IsSet()) {
    return false;
  } else if (!s1.IsSet() && !s2.IsSet()) {
    return false;
  } else if (s1.Get().size() < s2.Get().size()) {
    return true;
  } else if (s1.Get().size() > s2.Get().size()) {
    return false;
  } else {
  .....
}

第一次检查时出错。应该写成:
if (!s1.IsSet() && s2.IsSet()) {

NCBI基因组工作台项目,C ++
CRef<CSeq_align> CNWAligner::Run(CScope &scope, const CSeq_loc &loc1,
                                 const CSeq_loc &loc2, bool trim_end_gaps)
{
  if ((!loc1.IsInt() && !loc1.IsWhole()) ||
      (!loc1.IsInt() && !loc1.IsWhole()))
  {
    NCBI_THROW(CException, eUnknown,
               "Only whole and interval locations supported");
  }
  ....
}

条件的第一行中繁殖,但随后的程序员赶紧和忘记更换LOC1LOC2

FlashDevelop项目,C#
public void SetPrices(....)
{
  UInt32 a0 = _choice.GetPrice0();
  UInt32 a1 = _choice.GetPrice1();
  UInt32 b0 = a1 + _choice2.GetPrice0();   // <=
  UInt32 b1 = a1 + _choice2.GetPrice1();
  ....
}

FreeCAD项目,C ++
inline void insEdgeVec(std::map<int,std::set<int> > &map,
                       int n1, int n2)
{
  if(n1<n2)
    map[n2].insert(n1);
  else
    map[n2].insert(n1);
};

无论条件如何,都将执行相同的操作。看起来很简单。您怎么能复制一条线却不能解决呢?能够。

LibreOffice Project,C ++
class SVX_DLLPUBLIC SdrMarkView : public SdrSnapView
{
  ....
  const Point& GetRef1() const { return maRef1; }
  const Point& GetRef2() const { return maRef1; }
  ....
};

经典复制粘贴错误。正确地:
const Point& GetRef2() const { return maRef2; }

LibreOffice Project,C ++
bool CmpAttr(
  const SfxPoolItem& rItem1, const SfxPoolItem& rItem2)
{
  ....
  ::boost::optional<sal_uInt16> oNumOffset1 =
        static_cast<const SwFmtPageDesc&>(rItem1).GetNumOffset();
  ::boost::optional<sal_uInt16> oNumOffset2 =
        static_cast<const SwFmtPageDesc&>(rItem1).GetNumOffset();
  ....
}

还有一个经典的复制粘贴错误:)。在一个地方1到2纠正了,在另一个地方他们忘记了。

LibreOffice Project,C ++
XMLTransformerOOoEventMap_Impl::XMLTransformerOOoEventMap_Impl(
        XMLTransformerEventMapEntry *pInit,
        XMLTransformerEventMapEntry *pInit2 )
{
  if( pInit )
    AddMap( pInit );
  if( pInit )
    AddMap( pInit2 );
}

将1替换为2并没有错误,只是没有将2添加到第二个条件中。

图12


您可能会有点累。因此,我建议做茶或咖啡,我们将继续与数字0、1和2接触

。Geant4软件项目,C ++语言
void G4VTwistSurface::GetBoundaryLimit(G4int areacode,
                                       G4double limit[]) const
{
  ....
  if (areacode & sC0Min1Max) {
     limit[0] = fAxisMin[0];
     limit[1] = fAxisMin[1];
  } else if (areacode & sC0Max1Min) {
     limit[0] = fAxisMax[0];
     limit[1] = fAxisMin[1];
  } else if (areacode & sC0Max1Max) {
     limit[0] = fAxisMax[0];
     limit[1] = fAxisMax[1];
  } else if (areacode & sC0Min1Max) {
     limit[0] = fAxisMin[0];
     limit[1] = fAxisMax[1];
  }
  ....
}

希望您听取建议后休息。准备在此代码中发现错误?

恭喜发现错误的读者。你很棒!

那些懒得搜寻的人,我也明白。审查这样的代码非常繁琐,并且希望以某种方式快速检查更有趣的内容。在这里,静态分析仪会帮上大忙,因为它们不会累。

错误是这两个检查是相同的:
if        (areacode & sC0Min1Max) {
} else if (areacode & sC0Min1Max) {

如果您学习该代码,则很明显,第一次检查是错误的。正确地:
if        (areacode & sC0Min1Min) {
} else if (areacode & sC0Max1Min) {
} else if (areacode & sC0Max1Max) {
} else if (areacode & sC0Min1Max) {

CryEngine V项目,C ++
bool
CompareRotation(const Quat& q1, const Quat& q2, float epsilon)
{
  return (fabs_tpl(q1.v.x - q2.v.x) <= epsilon)
      && (fabs_tpl(q1.v.y - q2.v.y) <= epsilon)
      && (fabs_tpl(q2.v.z - q2.v.z) <= epsilon) // <=
      && (fabs_tpl(q1.w - q2.w) <= epsilon);
}

TortoiseGit项目,C ++
void CGitStatusListCtrl::OnContextMenuList(....)
{
  ....
  if( (!this->m_Rev1.IsEmpty()) ||
      (!this->m_Rev1.IsEmpty()) )
  ....
}

Geant4软件项目,C ++语言
G4double G4MesonAbsorption::
GetTimeToAbsorption(const G4KineticTrack& trk1,
                    const G4KineticTrack& trk2)
{
  ....
  if(( trk1.GetDefinition() == G4Neutron::Neutron() ||
       trk1.GetDefinition() == G4Neutron::Neutron() ) &&
       sqrtS>1.91*GeV && pi*distance>maxChargedCrossSection)
    return time;
  ....
}

MonoDevelop项目,C#
private bool MembersMatch(ISymbol member1, ISymbol member2)
{
  ....
  if (member1.DeclaredAccessibility !=
      member1.DeclaredAccessibility
   || member1.IsStatic != member1.IsStatic)
  {
    return false;
  }
  ....
}

如您所见,虽然代码片段没有说明。实际上,这里没有什么要解释的。你只能叹气。

海豚模拟器项目,C ++
bool IRBuilder::maskedValueIsZero(InstLoc Op1, InstLoc Op2) const
{
  return (~ComputeKnownZeroBits(Op1) &
          ~ComputeKnownZeroBits(Op1)) == 0;
}

RunAsAdmin Explorer Shim项目,C ++
bool IsLuidsEqual(LUID luid1, LUID luid2)
{
  return (luid1.LowPart == luid2.LowPart) &&
         (luid2.HighPart == luid2.HighPart);
}

IT ++项目,C ++语言
Gold::Gold(const ivec &mseq1_connections,
           const ivec &mseq2_connections)
{
  ....
  it_assert(mseq1.get_length() == mseq1.get_length(),
            "Gold::Gold(): dimension mismatch");
}

QuantLib项目,C ++
Distribution ManipulateDistribution::convolve(
  const Distribution& d1, const Distribution& d2) {
  ....
  QL_REQUIRE (d1.xmin_ == 0.0 && d1.xmin_ == 0.0,
              "distributions offset larger than 0");
  ....
}

Samba项目,C ++
static bool samu_correct(struct samu *s1, struct samu *s2)
{
  ....
  } else if (s1_len != s1_len) {
    DEBUG(0, ("Password history not written correctly, "
              "lengths differ, want %d, got %d\n",
          s1_len, s2_len));
  ....
}

Mozilla Firefox项目,C ++
static PRBool IsZPositionLEQ(nsDisplayItem* aItem1,
                             nsDisplayItem* aItem2,
                             void* aClosure) {
  if (!aItem1->GetUnderlyingFrame()->Preserves3D() ||
      !aItem1->GetUnderlyingFrame()->Preserves3D()) {
    return IsContentLEQ(aItem1, aItem2, aClosure);
  }
  ....
}

Haiku操作系统项目,C ++
void trans_double_path::reset()
{
  m_src_vertices1.remove_all();
  m_src_vertices2.remove_all();
  m_kindex1 = 0.0;               // <=
  m_kindex1 = 0.0;               // <=
  m_status1 = initial;
  m_status2 = initial;
}

Qt项目,C ++

好,现在让我们变得更加复杂。为了好玩,请尝试在此处查找错误:
static ShiftResult shift(....)
{
  ....
  qreal l = (orig->x1 - orig->x2)*(orig->x1 - orig->x2) +
            (orig->y1 - orig->y2)*(orig->y1 - orig->y1) *
            (orig->x3 - orig->x4)*(orig->x3 - orig->x4) +
            (orig->y3 - orig->y4)*(orig->y3 - orig->y4);
  ....
}

一张图片,以免立刻看到答案,并有机会思考。

图13


没错,应该写成orig-> y1-orig-> y2而不是orig-> y1-orig-> y1.NET编译器平台项目,C#语言


public void IndexerMemberRace()
{
  ....
  for (int i = 0; i < 20; i++)
  {
    ....
    if (i % 2 == 0)
    {
      thread1.Start();
      thread2.Start();
    }
    else
    {
      thread1.Start();
      thread2.Start();
    }
    ....
  }
  ....
}

有趣的情况。为了进行测试,您需要以不同的顺序运行线程。但是,由于输入错误,线程始终以相同的方式启动,因此测试所检查的内容少于应有的方式。

正确地:
if (i % 2 == 0)
{
  thread1.Start();
  thread2.Start();
}
else
{
  thread2.Start();
  thread1.Start();
}

Samba项目,C语言
static int compare_procids(const void *p1, const void *p2)
{
  const struct server_id *i1 = (struct server_id *)p1;
  const struct server_id *i2 = (struct server_id *)p2;

  if (i1->pid < i2->pid) return -1;
  if (i2->pid > i2->pid) return 1;
  return 0;
}

比较函数将永远不会返回1,因为条件i2-> pid> i2-> pid没有意义。

自然地,这是普通的错字,实际上应该写成:
if (i1->pid > i2->pid) return 1;

ChakraCore项目,C ++

本章的最后一种情况。万岁!
bool Lowerer::GenerateFastBrSrEq(....,
                                 IR::RegOpnd * srcReg1,
                                 IR::RegOpnd * srcReg2,
                                 ....)
{
  ....
  else if (srcReg1 && (srcReg1->m_sym->m_isStrConst))
  ....
  else if (srcReg1 && (srcReg1->m_sym->m_isStrConst))
  ....
}


其他错误


现在,让我们讨论与数字0、1、2相关的错误模式。

在明确使用常数0/1/2的条件下的错字


C + +根项目
Int_t TProofMonSenderML::SendSummary(TList *recs, const char *id)
{
  ....
  if (fSummaryVrs == 0) {
    if ((dsn = recs->FindObject("dataset"))) recs->Remove(dsn);
  } else if (fSummaryVrs == 0) {
  ....
}

两次比较fSummaryVrs与0 很奇怪

。.NETCoreCLR项目,C#
void PutIA64Imm22(UINT64 * pBundle, UINT32 slot, INT32 imm22)
{
  if (slot == 0)             // <=
  {
    ....
  }
  else if (slot == 1)
  {
    ....
  }
  else if (slot == 0)        // <=
  {
    .... 
  }
  ....
}

FFmpeg项目,C语言
static int imc_decode_block(....)
{
  ....
  if (stream_format_code & 0x1)
    imc_decode_level_coefficients_raw(....);
  else if (stream_format_code & 0x1)
    imc_read_level_coeffs_raw(....);
  ....
}


邮编/名称


之前我们曾考虑过索引或名称不正确的情况。在这种情况下,您不会立即说出如何对错误进行分类。这个例子可以归于一章和另一章。因此,我决定将其分开出售。

Mesa 3D图形库项目,C ++
bool
ir_algebraic_visitor::reassociate_constant(....)
{
  ....
  if (ir1->operands[0]->type->is_matrix() ||
      ir1->operands[0]->type->is_matrix() ||
      ir2->operands[1]->type->is_matrix() ||
      ir2->operands[1]->type->is_matrix())
   return false;
  ....
}

此代码可以像这样固定:
if (ir1->operands[0]->type->is_matrix() ||
    ir1->operands[1]->type->is_matrix() ||
    ir2->operands[0]->type->is_matrix() ||
    ir2->operands[1]->type->is_matrix())

您可以像这样修复它:
if (ir1->operands[0]->type->is_matrix() ||
    ir2->operands[0]->type->is_matrix() ||
    ir1->operands[1]->type->is_matrix() ||
    ir2->operands[1]->type->is_matrix())


额外0


有时0是多余且有害的。因此,该数字可以在不需要时变成八进制。或破坏格式字符串。

提到的错误不适合本文,但我认为值得一提。在本文中,我不会给出包含这些错误的代码,但是如果您有兴趣,可以在这里查看它们:
  • V536请注意,使用的常数以八进制形式表示,例如
  • V638字符串内存在终端空值。遇到'\ 0xNN'字符。可能的意思是:'\ xNN',例子


忘了写+1


Haiku操作系统项目,C ++
int
UserlandFS::KernelEmu::new_path(const char *path, char **copy)
{
  ....
  // append a dot, if desired
  if (appendDot) {
    copiedPath[len] = '.';
    copiedPath[len] = '\0';
  }
  ....
}

正确的选项:
copiedPath[len] = '.';
copiedPath[len + 1] = '\0';

注意。他们忘记添加单位的情况并不罕见。我确实记得我遇到过这种情况不止一次。但是,当我想为本文键入类似的示例时,我只找到了这个示例。很抱歉,我无法再用这些错误吓到您。我道歉。

格式化错误(C#)


通常,用于构造字符串的函数使用少量参数。因此,事实证明,错误通常与使用{0},{1}或{2}有关。

Azure PowerShell项目,C#
protected override void ProcessRecordInternal()
{
  ....
  if (this.ShouldProcess(this.Name,
    string.Format("Creating Log Alert Rule '{0}' in resource group {0}",
      this.Name, this.ResourceGroupName)))
  {
    ....
  }
  ....
}

查封并写两次{0}。结果,名称this.Name将被两次插入到字符串中但是名称this.ResourceGroupName不会进入创建的字符串中。

Mono项目,C#
void ReadEntropy ()
{
  if (reader.IsEmptyElement)
    throw new XmlException (
      String.Format ("WS-Trust Entropy element is empty.{2}",
                      LineInfo ()));
  ....
}

这通常很奇怪。您需要插入不是的内容。此代码很可能未成功进行重构,结果被破坏了。

Xenko项目,C#
public string ToString(string format,
                                IFormatProvider formatProvider)
{
  if (format == null)
    return ToString(formatProvider);

  return string.Format(
                      formatProvider,
                      "Red:{1} Green:{2} Blue:{3}",
                      R.ToString(format, formatProvider),
                      G.ToString(format, formatProvider),
                      B.ToString(format, formatProvider));
}

程序员忘记编号从{0}开始,而不是{1}。正确的代码是:
return string.Format(
                    formatProvider,
                    "Red:{0} Green:{1} Blue:{2}",
                    R.ToString(format, formatProvider),
                    G.ToString(format, formatProvider),
                    B.ToString(format, formatProvider));

.NET编译器平台项目,C#
private void DumpAttributes(Symbol s)
{
  ....
  Console.WriteLine("{0} {1} {2}", pa.ToString());
  ....
}

争论显然是不够的。

结论与建议


我不得不演示很多例子,以显示与0、1和2有关的错别字值得特别注意。

如果我只是简单地说:“容易混淆o1和o2”,则您会同意,但在阅读或至少滚动浏览该文章后,现在不会附加您现在所附加的含义。

现在警告您,这很好。有备则无患。现在,您将更加专心于代码审查,并特别注意变量,在变量名称中您将看到0、1、2。

很难就如何避免这种错误设计代码提出一些建议。如您所见,即使在没有问题的简单代码中也发现了错误。

因此,我不会敦促您避免使用0、1、2并给变量起长名。如果您开始写First / Second / Left / Right等数字而不是数字,那么复制名称或表达式的诱惑将更大。也许这样的建议最终不会减少,但是会增加错误的数量。

但是,当您编写很多相同类型的代码时,“表格代码设计”的建议仍然有意义。表格格式不能保证没有错字,但会使它们更容易,更快地注意到。请参阅迷你书“ 编程,重构以及所有这些的主要问题 ”中 N13章

还有一个好消息。使用PVS-Studio静态代码分析器可以找到本文中讨论的所有错误。因此,在开发过程中引入静态分析工具后,您可以在早期识别出许多错别字。

谢谢您的关注。我希望你有兴趣和害怕。希望您能获得可靠的代码,并减少0、1、2的错误,以使Freddy不会来找您。



如果您想与讲英语的读者分享这篇文章,请使用翻译链接:Andrey Karpov。零,一,二,弗雷迪来找你

All Articles