Get best of the week

Cero, uno, dos, Freddy te recogerá

Foto 1

Aquí hay una continuación de una serie de artículos que pueden titularse "horrores para programadores". Esta vez hablaremos sobre un patrón típico de error tipográfico asociado con el uso de los números 0, 1, 2. No importa si escribes en C, C ++, C # o Java. Si usa las constantes 0, 1, 2, o si estos números están contenidos en nombres de variables, entonces, muy probablemente, Freddy lo visitará por la noche. Lea y no diga más tarde que no fue advertido.


Introducción


Continúo una serie de artículos dedicados a patrones notados en cómo las personas cometen errores. Publicaciones anteriores:
  1. Efecto de última línea
  2. La función más peligrosa en el mundo de C / C ++
  3. El mal vive en funciones de comparación

Esta vez no noté el patrón, sino mi colega Svyatoslav Razmyslov. Llamó la atención sobre el hecho de que constantemente describe en sus artículos problemas en los que aparecen en su nombre variables que contienen los números 1 y 2. Svyatoslav me sugirió que estudiara este tema con más detalle y realmente resultó ser muy fructífero. Resultó que nuestra colección de errores contiene una gran cantidad de fragmentos de código que son incorrectos debido al hecho de que las personas se confunden en los índices 0, 1, 2 o en los nombres de variables que contienen dichos números. Se revela una nueva regularidad interesante, que se discutirá a continuación. Estoy agradecido a Svyatoslav por la solicitud de investigar este tema y dedicarle este artículo.

Figura 14

Svyatoslav Razmyslov, gerente, atento recolector de insectos y solo una persona talentosa.

¿Cuál es el propósito de este artículo? Muestre lo fácil que todos cometemos errores y errores tipográficos. Si se advierte a los programadores, estarán más atentos en el proceso de revisiones de código, enfocándose en el malogrado 0, 1, 2. Además, los programadores podrán sentir mejor el valor de los analizadores de código estático que ayudan a notar tales errores. No se trata de publicidad PVS-Studio (aunque también es :). Hasta ahora, muchos programadores encuentran superfluo el análisis estático, prefiriendo concentrarse en su propia precisión y revisiones de código. Desafortunadamente, intentar escribir código sin errores es bueno, pero no suficiente. Este artículo demostrará una vez más esto claramente.

Nadie es inmune a tales errores. A continuación, verá bloopers épicos incluso en proyectos tan conocidos como Qt, Clang, Hive, LibreOffice, Linux Kernel, .NET Compiler Platform, XNU kernel, Mozilla Firefox. Y estos no son algunos errores raros exóticos, sino los más frecuentes. ¿No convencido? Entonces comencemos!

“¡Chatter no vale nada! ¡Muéstrame los errores!

© cita modificada por Linus Torvalds.

Errores tipográficos en constantes al indexar matrices


Como regla general, en nuestros artículos damos advertencias con la ayuda de qué errores se encuentran. Esta vez omitiré estas advertencias, ya que sin ellas el error será fácilmente perceptible y comprensible. Sin embargo, aunque estos errores son evidentes de inmediato en un código corto, saben cómo esconderse en el código de los proyectos.

Comencemos con los errores cuando haya confusión con los literales numéricos utilizados para indexar matrices. A pesar de la banalidad de estos errores, hay muchos de ellos y no se revelan en el trabajo de laboratorio de los estudiantes.

Proyecto de núcleo XNU, lenguaje 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;
  }
  ....
}

La línea se copió, pero olvidó corregir el índice. Según tengo entendido, debería escribirse aquí:
wrap.Seal_Alg[0] = 0xff;
wrap.Seal_Alg[1] = 0xff;

Proyecto LibreOffice, 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;
}

Como en el caso anterior, la línea se copió, pero olvidó corregir 0 por 1. Solo se corrigió el literal de cadena.

Uno puede hacer una pregunta filosófica, ¿cómo puede uno cometer tal error en una función de cuatro líneas? Todo es posible. Aquí está, programación.

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

En la línea copiada, olvidaron reemplazar dir [1] con dir [2] . Como resultado, el valor a lo largo del eje Z no se controla.

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

Sí, incluso en un constructor tan corto, puede lograr ir más allá del límite de una matriz cuando se inicializa.

Figura 8


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

No se requiere comentario.

Proyecto asterisco, lenguaje 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;
  ....
}

Al escribir el mismo tipo de bloques, el error, como regla, se encuentra en el subyacente. Antes de esto, todos los casos considerados eran solo eso. Aquí el error tipográfico está en un lugar inusual, es decir, en el primer bloque. Por qué sucedió es difícil de decir. No tengo más remedio que traer una imagen de un unicornio encogiéndose de hombros:

Figura 9


Proyecto de tecnología CASCADE abierto, C ++
inline void Prepend(const Standard_Integer theIndex)
{
  if (myIndex[1] >= 0)
    Standard_OutOfRange::Raise ("BRepMesh_PairOfIndex....");

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

Dos veces en la misma celda de la matriz, se copian diferentes valores. Un error obvio, pero cómo solucionarlo no estaba claro para mí, ya que el código del proyecto no me es familiar. Así que solo miré cómo los desarrolladores corrigieron el código después de que nuestro equipo les señaló este error. La opción correcta:
myIndex[1] = myIndex[0];
myIndex[0] = theIndex;

Proyecto de tubería transproteómica, 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;
  }
  ....
}

Me preocupa que los paquetes de investigación contengan tales errores. La tubería transproteómica está diseñada para resolver problemas en el campo de la biología. Esto se puede decidir e "investigar". Este paquete generalmente encuentra muchas cosas interesantes: verifique en 2012 , verifique 2013 . Quizás pueda intentar nuevamente mirar este proyecto.

Proyecto ITK, lenguaje C ++

Nos enfrentamos a otro proyecto para llevar a cabo investigaciones en el campo de la medicina: el Kit de herramientas de registro y segmentación de medicina Insight (ITK). El proyecto es diferente, pero los errores son los mismos.
template< typename TCoordRepType >
void
VoronoiDiagram2D< TCoordRepType >::SetOrigin(PointType vorsize)
{
  m_VoronoiBoundaryOrigin[0] = vorsize[0];
  m_VoronoiBoundaryOrigin[0] = vorsize[1];
}

Proyecto ITK, C ++


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

Limpiar copiar y pegar.

Proyecto 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);
  ....
}

Lo más probable es que la constante crBackgnd se escriba en la celda cols [2] .

Proyecto 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))) {
  ....
}

El elemento de la matriz de tamaño [1] se verifica dos veces , y el elemento de tamaño [2] no se verifica. Así es como aparecen artefactos extraños en las imágenes.

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

Se siente directamente que la expresión cmptlut [0] <0 se duplicó dos veces al copiar, pero se corrigió el cero en un solo lugar.

Proyecto de kit de herramientas de visualización (VTK), C ++
void vtkImageStencilRaster::PrepareForNewData(....)
{
  ....
  if (allocateExtent &&
      allocateExtent[1] >= allocateExtent[1])
  ....
}

De aquí en adelante, no comentaré muchos de estos errores. ¿Qué hay para comentar? Lo principal al mirar dichos fragmentos de código es sentir que aunque el error es simple, esto no significa que el programador lo notará.

Proyecto de kit de herramientas de visualización (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;
  ....
}

Aquí, el programador claramente tenía prisa por escribir código más rápido. Es difícil explicar de otra manera cómo cometió un error tres veces. Los elementos de la matriz se restan de sí mismos. El resultado es que este código es equivalente:
inZPtr +=
  (0)*inIncs[0] * data_type_size +
  (0)*inIncs[1] * data_type_size +
  (0)*inIncs[2] * data_type_size;

Sin embargo, este código se puede reducir aún más:
inZPtr += 0;

Suntuosamente. Hay una expresión larga y seria en el código que, de hecho, no hace nada. Amo tales casos.

Proyecto Visualization Toolkit (VTK), lenguaje C ++

Un caso similar de código de escritura apresurado.
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);
    }
}

La comparación newPos [2]! = OldPos [2] se repite dos veces .

Proyecto de entorno de comunicación adaptable (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);
  }
  ....
}

La condición debe verificar que se encuentren dos barras después de los dos puntos. En otras palabras, se busca la subcadena ": //". Debido a un error tipográfico, el cheque es "ciego" y está listo para contar cualquier personaje como un segundo corte.

Proyecto de muestras 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]));
  ....
}

Aquí hay un error tipográfico, en los argumentos pasados ​​a la macro:
IPP_MIN(m_cur.AcRate[2],m_cur.AcRate[2])

Resulta que se selecciona un mínimo de dos valores idénticos. De hecho, debería escribirse:
IPP_MIN(m_cur.AcRate[2],m_cur.AcRate[3])

Por cierto, este código puede demostrar la utilidad de la biblioteca estándar. Si escribes así:
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));

Ese código será más corto y menos propenso a errores. En realidad, cuanto menos sea el mismo tipo de código, es más probable que se escriba correctamente.

Proyecto 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]);
  ....
}

La expresión correcta es:
int ztrend = sgn(buffer[samplesleft - WindowSizeInt-2]-
                 buffer[samplesleft - WindowSizeInt-1]);

Proyecto PDFium, lenguaje 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;
}

Se duplica una serie de acciones para inicializar la estructura. Las líneas marcadas con el comentario // 2 se pueden eliminar y nada cambiará. Dudaba si incluir este fragmento de código en el artículo. Esto no es un gran error, y no del todo con los índices. Sin embargo, este código adicional probablemente apareció precisamente porque el programador se confundió en todos estos miembros de clase e índices 1, 2. Entonces, creo que este código es adecuado para demostrar cuán fácil es confundirse en números.

Proyecto CMake, C

El código que se describe a continuación no fue escrito por los desarrolladores de CMake, sino que fue prestado. A juzgar por el comentario al comienzo del archivo, la función utf8_encodeFue escrito por Tim Kientzle en 2007. Desde entonces, esta función ha estado vagando de un proyecto a otro, y hay muchos de ellos. No estudié el tema de la fuente original, ya que este no es el punto ahora. Dado que este código está en el proyecto CMake, el error se aplica a 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;
  ....
}

Como puede ver, hay algún tipo de confusión con los índices. Dos veces hay un registro en el elemento de matriz p [1] . Si estudia el código en el vecindario, queda claro que el código correcto debería ser así:
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;

Nota

Tenga en cuenta que todos los errores discutidos en este capítulo están relacionados con el código C o C ++. ¡Sin código C # o Java!

Esto es muy interesante, no esperaba esto. En mi opinión, los errores tipográficos considerados no dependen del idioma. Y en los siguientes capítulos, aparecerán errores en el código en otros idiomas. Creo que esto es solo una coincidencia. El analizador PVS-Studio comenzó mucho más tarde para admitir lenguajes C # / Java en comparación con C / C ++, y simplemente no logramos acumular los ejemplos de error correspondientes en la base de datos.

Sin embargo, la observación sigue siendo interesante. Aparentemente, a los programadores de C y C ++ les gusta usar los números 0, 1 y 2 más cuando trabajan con matrices :).

Errores ortográficos en los nombres


Esta será la sección más grande. Es muy fácil para las personas confundirse con nombres como a1 y a2 . Parece que te puedes confundir aquí? Lata. Fácil. Y ahora el lector podrá verificar esto.

Proyecto Colmena, 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;
}

La función de comparación compare toma dos objetos: o1 y o2 . Pero debido a un error tipográfico, solo se usa más o2 .

Curiosamente, gracias a Copy-Paste, este error migró a otra función:
@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;
}

Figura 10


Proyecto Infer.NET, lenguaje C #
private void MergeParallelTransitions()
{
  ....
  if (double.IsInfinity(transition1.Weight.Value) &&    
      double.IsInfinity(transition1.Weight.Value))
  ....
}

Proyecto Doom 3, C ++
uint AltOp::fixedLength()
{
  uint l1 = exp1->fixedLength();
  uint l2 = exp1->fixedLength();

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

  return l1;
}

Si alguien no notó inmediatamente un error tipográfico, entonces debe mirar la línea donde se inicializa la variable l2 . Debería usar exp2 .

Proyecto 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;
  }
  ....
}

Correctamente:
nFPSThreshold1 = 60;
nFPSThreshold2 = 50;

Proyecto Kernel Linux, lenguaje C

Por cierto, los errores tipográficos pueden estar no solo en nombres de variables, sino también en nombres de macro. Ahora habrá varios ejemplos de este tipo.
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;
  }
  ....
}

Como puede ver, una máscara llamada BIT1 se usa dos veces , lo que hace que la segunda verificación no tenga sentido. El cuerpo de la segunda declaración condicional comentada nunca se ejecutará.

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

Se planeó un comportamiento aleatorio en el juego, pero siempre se selecciona la misma constante SAY_BELNISTRASZ_AGGRO_1 .

Proyecto Vangers: One For The Road, 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;
  }
  ....
}

A juzgar por el código escrito al lado, la opción correcta debería ser así:
ret = (lang)
  ? iJoystickStickSwitch2[vkey - VK_STICK_SWITCH_1]
  : iJoystickStickSwitch1[vkey - VK_STICK_SWITCH_1];

Proyecto RT-Thread, lenguaje 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 es un sistema operativo de código abierto en tiempo real para dispositivos integrados. Aquí vemos la confusión entre FIFO 0 y FIFO 1. Y en algún lugar, alguien encontrará un dispositivo con errores.

Figura 11


El error está aquí:
if      (CAN_FIFO0 == fifo_number){
....
}else if(CAN_FIFO0 == fifo_number){

El segundo cheque siempre da falso. Correctamente:
if      (CAN_FIFO0 == fifo_number){
....
}else if(CAN_FIFO1 == fifo_number){

Proyecto Colmena, 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")) {
    ....
}

El analizador PVS-Studio indica inmediatamente 2 errores:
  1. Una cadena almacenada en colOrScalar1 no puede ser igual a las cadenas Col y Column;
  2. La cadena almacenada en colOrScalar1 no puede ser igual a las cadenas Col y Scalar al mismo tiempo.

Existe una clara confusión sobre los nombres de variables.

Proyecto Shareaza, lenguaje 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()))
    ....
}

Correctamente:
pAttr1->GetValue().CompareNoCase(pAttr2->GetValue())

Nota

Hagamos una breve pausa. Existe el temor de que al mirar a través de una montaña de errores banales, olvidaremos por qué estamos haciendo esto.

La tarea no es reírse del código de otra persona. Todo esto no es una razón para tocar su dedo y decir: "Ja, ja, bueno, tienes que hacerlo". Esta razón para pensar!

Las publicaciones de nuestro equipo están diseñadas para mostrar que ninguno de nosotros es inmune a los errores. Los errores descritos en el artículo aparecen en el código con mucha más frecuencia de lo que cabría esperar. También es importante que la probabilidad de perderse en 0, 1, 2 casi no dependa de las calificaciones del programador.

Es útil darse cuenta de que las personas tienden a cometer errores. Sin esto, no puede dar el siguiente paso para mejorar la calidad y confiabilidad del código. Al comprender que todos estamos equivocados, las personas comienzan a tratar de identificar errores en las primeras etapas, utilizando estándares de codificación, revisiones de códigos, pruebas unitarias, analizadores estáticos y dinámicos. Es muy bueno.

¿Por qué se escriben cosas tan entendibles? Desafortunadamente, al comunicarnos con una gran cantidad de desarrolladores, nos vemos obligados a afirmar que no siempre es tan claro para todos. Muchos tienen una autoestima demasiado alta y simplemente no permiten pensar que son capaces de cometer errores simples. Es triste.

Si usted es un líder / gerente de equipo, lo invito a que se familiarice con esta nota al mismo tiempo .

Proyecto 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())
  ....
}

Correctamente:
const Numeric *const num2 = o2.as<Numeric>();

Proyecto de Android, lenguaje 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;
}

Dos errores tipográficos a la vez, debido a que las variables pr2.mStretchMode y pr2.mFallbackMode se comparan entre sí.

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

Al final, sellaron y dividieron la variable p1.z misma.

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

Sí, sí, el analizador PVS-Studio encuentra errores similares en los compiladores. Correctamente:
(Ty1->isIntegerType() &&
 Ty2->isIntegerType())

Proyecto 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())) {
  ....
}

Correctamente:
(Op0I->hasOneUse() || Op1I->hasOneUse())

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

Proyecto NCBI Genome Workbench, 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 {
  .....
}

Error en la primera verificación. Debería estar escrito:
if (!s1.IsSet() && s2.IsSet()) {

Proyecto NCBI Genome Workbench, 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");
  }
  ....
}

La primera línea de la condición se propagó, pero luego el programador se apresuró y olvidó reemplazar loc1 por loc2 .

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

Proyecto 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);
};

Independientemente de la condición, se realiza la misma acción. Parecería un caso tan simple. ¿Cómo podrías copiar una línea y no arreglarla? Lata.

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

Error clásico de copiar y pegar. Correctamente:
const Point& GetRef2() const { return maRef2; }

Proyecto LibreOffice, 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();
  ....
}

Y otro error clásico de copiar y pegar :). En un lugar se corrigieron 1 a 2, y en otro se olvidaron.

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

No hay ningún error en reemplazar 1 con 2, pero simplemente no agregó 2 a la segunda condición.

Figura 12


Puede que estés un poco cansado. Por lo tanto, propongo hacer té o café, y continuaremos conociendo el mundo de los números 0, 1 y 2.

Proyecto de software Geant4, lenguaje 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];
  }
  ....
}

Espero que hayas tomado el consejo y descansado. ¿Listo para encontrar un error en este código?

Felicitaciones a los lectores que han notado un error. ¡Eres genial!

Aquellos que son demasiado flojos para buscar, también los entiendo. La revisión de dicho código es muy tediosa y existe el deseo de verificar rápidamente algo más interesante. Aquí es donde los analizadores estáticos ayudan mucho porque no se cansan.

El error es que estas dos comprobaciones son las mismas:
if        (areacode & sC0Min1Max) {
} else if (areacode & sC0Min1Max) {

Si estudia el código, queda claro que la primera comprobación es errónea. Correctamente:
if        (areacode & sC0Min1Min) {
} else if (areacode & sC0Max1Min) {
} else if (areacode & sC0Max1Max) {
} else if (areacode & sC0Min1Max) {

Proyecto 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);
}

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

Proyecto de software Geant4, lenguaje 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;
  ....
}

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

Como puede ver, mientras que los fragmentos de código van sin explicación. En realidad, no hay nada que explicar aquí. Solo puedes suspirar.

Proyecto Dolphin Emulator, C ++
bool IRBuilder::maskedValueIsZero(InstLoc Op1, InstLoc Op2) const
{
  return (~ComputeKnownZeroBits(Op1) &
          ~ComputeKnownZeroBits(Op1)) == 0;
}

Proyecto de cuña RunAsAdmin Explorer, C ++
bool IsLuidsEqual(LUID luid1, LUID luid2)
{
  return (luid1.LowPart == luid2.LowPart) &&
         (luid2.HighPart == luid2.HighPart);
}

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

Proyecto 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");
  ....
}

Proyecto 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));
  ....
}

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

Proyecto del sistema operativo 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;
}

El proyecto Qt, C ++

Ok, ahora empecemos un poco más. Por diversión, intenta encontrar el error aquí:
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);
  ....
}

Una imagen, para no ver la respuesta de inmediato, y tuve la oportunidad de pensar.

Figura 13


Así es, en lugar de orig-> y1 - orig-> y1 debería escribirse orig-> y1 - orig-> y2 .

.NET Compiler Platform Project, lenguaje C #
public void IndexerMemberRace()
{
  ....
  for (int i = 0; i < 20; i++)
  {
    ....
    if (i % 2 == 0)
    {
      thread1.Start();
      thread2.Start();
    }
    else
    {
      thread1.Start();
      thread2.Start();
    }
    ....
  }
  ....
}

Interesante caso. Para fines de prueba, debe ejecutar subprocesos en un orden diferente. Sin embargo, debido a un error tipográfico, los subprocesos siempre comienzan de la misma manera, como resultado de lo cual la prueba comprueba menos de lo que debería.

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

Proyecto Samba, lenguaje 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;
}

La función de comparación nunca devolverá 1, ya que la condición i2-> pid> i2-> pid no tiene sentido.

Naturalmente, este es un error tipográfico ordinario, y de hecho debería escribirse:
if (i1->pid > i2->pid) return 1;

Proyecto ChakraCore, C ++

El último caso en este capítulo. ¡Hurra!
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))
  ....
}


Otros errores


Ahora hablemos de patrones de error menos numerosos asociados con el uso de los números 0, 1, 2.

Errores tipográficos en condiciones donde la constante 0/1/2 se usa explícitamente


Proyecto ROOT, 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) {
  ....
}

Es extraño comparar fSummaryVrs con 0. dos veces .

Proyecto .NET CoreCLR, C #
void PutIA64Imm22(UINT64 * pBundle, UINT32 slot, INT32 imm22)
{
  if (slot == 0)             // <=
  {
    ....
  }
  else if (slot == 1)
  {
    ....
  }
  else if (slot == 0)        // <=
  {
    .... 
  }
  ....
}

Proyecto FFmpeg, lenguaje 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(....);
  ....
}


Zip / Nombre


Anteriormente consideramos casos en los que el índice o el nombre son incorrectos. Y aquí hay una situación en la que no dirá inmediatamente cómo clasificar el error. Este ejemplo podría atribuirse tanto a uno como al otro capítulo. Por lo tanto, decidí traerlo por separado.

Proyecto de biblioteca de gráficos 3D de Mesa, 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;
  ....
}

Este código se puede arreglar así:
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())

Y puedes arreglarlo así:
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 extra


Algunas veces 0 es redundante y dañino. Por eso, el número puede convertirse en octal donde no es necesario. O estropear la cadena de formato.

Los errores mencionados no son adecuados para este artículo, pero creo que vale la pena mencionarlos. No daré un código con estos errores en el artículo, pero si está interesado, puede verlos aquí:
  • V536 Tenga en cuenta que el valor constante utilizado está representado por una forma octal, ejemplos ;
  • V638 Un terminal nulo está presente dentro de una cadena. Se encontraron los caracteres '\ 0xNN'. Probablemente significaba: '\ xNN', ejemplos .


Olvidé escribir +1


Proyecto del sistema operativo Haiku, C ++
int
UserlandFS::KernelEmu::new_path(const char *path, char **copy)
{
  ....
  // append a dot, if desired
  if (appendDot) {
    copiedPath[len] = '.';
    copiedPath[len] = '\0';
  }
  ....
}

La opción correcta:
copiedPath[len] = '.';
copiedPath[len + 1] = '\0';

Nota. La situación en la que olvidaron agregar una unidad no es rara. Recuerdo exactamente que más de una vez conocí tales casos. Sin embargo, cuando quería escribir ejemplos similares para el artículo, solo encontré este ejemplo. Lamento no poder asustarte más con estos errores. Me disculpo.

Errores de formato (C #)


Muy a menudo, las funciones para construir cadenas operan con un pequeño número de argumentos. Por lo tanto, resulta que los errores más frecuentes están asociados con el uso de {0}, {1} o {2}.

Proyecto 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)))
  {
    ....
  }
  ....
}

Sellado y escribió {0} dos veces. Como resultado, el nombre this.Name se insertará en la cadena dos veces . Pero el nombre this.ResourceGroupName no entrará en la cadena creada.

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

Esto es generalmente extraño. Necesita insertar lo que no es. Lo más probable es que este código se sometió a una refactorización fallida y resultó estar roto.

Proyecto 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));
}

El programador olvidó que la numeración comienza con {0}, no {1}. El código correcto es:
return string.Format(
                    formatProvider,
                    "Red:{0} Green:{1} Blue:{2}",
                    R.ToString(format, formatProvider),
                    G.ToString(format, formatProvider),
                    B.ToString(format, formatProvider));

.NET Compiler Platform Project, C #
private void DumpAttributes(Symbol s)
{
  ....
  Console.WriteLine("{0} {1} {2}", pa.ToString());
  ....
}

Los argumentos claramente no son suficientes.

Conclusiones y Recomendaciones


Tuve que demostrar tantos ejemplos para mostrar que los errores tipográficos relacionados con 0, 1 y 2 merecen especial atención.

Si simplemente dijera: "Es fácil confundir o1 y o2", estaría de acuerdo, pero no le atribuiría el significado que le atribuye ahora al leer o al menos desplazarse por el artículo.

Ahora estás advertido, y eso es bueno. Prevenido vale por dos. Ahora estará más atento a las revisiones de código y prestará más atención a las variables, en cuyos nombres verá 0, 1, 2. Es

difícil dar recomendaciones sobre el diseño del código para evitar tales errores. Como ha visto, los errores se encuentran incluso en un código tan simple, donde no hay nada que emitir.

Por lo tanto, no lo instaré a evitar 0, 1, 2 y dar nombres largos a las variables. Si en lugar de números comienza a escribir Primero / Segundo / Izquierda / Derecha, etc., la tentación de copiar el nombre o la expresión será aún mayor. Quizás tal recomendación finalmente no reducirá, sino que aumentará el número de errores.

Sin embargo, cuando escribe mucho del mismo tipo de código, la recomendación del "diseño de código tabular" sigue siendo relevante. El formato tabular no garantiza la ausencia de errores tipográficos, pero los hace más fáciles y rápidos de notar. Vea el capítulo N13 en el mini libro " La cuestión principal de la programación, refactorización y todo eso ".

Hay una buena noticia más. Todos los errores discutidos en este artículo se encontraron usando el analizador de código estático PVS-Studio .. En consecuencia, al introducir herramientas de análisis estático en el proceso de desarrollo, puede identificar muchos errores tipográficos en una etapa temprana.

Gracias por la atención. Espero que estés interesado y asustado. Le deseo un código confiable y menos errores con 0, 1, 2, para que Freddy no se acerque a usted.



Si desea compartir este artículo con una audiencia de habla inglesa, utilice el enlace a la traducción: Andrey Karpov. Cero, uno, dos, Freddy viene por ti .

More articles:


All Articles