Fragmento tomado de ReactOS proyecto (sistema operativo de código abierto compatible con Windows). El error se detecta mediante el siguiente diagnóstico:V560 Una parte de la expresión condicional siempre es verdadera:10035L.
void adns__querysend_tcp(adns_query qu, struct timeval now) {
...
if (!(errno == EAGAIN || EWOULDBLOCK ||
errno == EINTR || errno == ENOSPC ||
errno == ENOBUFS || errno == ENOMEM)) {
...
} Explicación
El ejemplo de código anterior es pequeño y puede detectar fácilmente el error en él. Pero cuando se trata de código de la vida real, los errores suelen ser muy difíciles de notar. Al leer un código como ese, tiendes a saltarte inconscientemente bloques de comparaciones similares y pasar al siguiente fragmento.
La razón por la que sucede, tiene que ver con que las condiciones están mal formateadas y no te apetece prestarles demasiada atención porque requiere cierto esfuerzo, y suponemos que como las comprobaciones son similares, apenas hay errores en la condición y todo debería estar bien.
Una de las formas de salir es formatear el código como una tabla.
Así que aquí falta "errno ==" en una de las comprobaciones. Da como resultado que la condición siempre sea verdadera ya que EWOULDBLOCK no es igual a cero.
Código correcto
if (!(errno == EAGAIN || errno == EWOULDBLOCK ||
errno == EINTR || errno == ENOSPC ||
errno == ENOBUFS || errno == ENOMEM)) { Recomendación
Para empezar, aquí hay una versión de este código formateado en el estilo de "tabla" más simple. En realidad, no nos gusta.
if (!(errno == EAGAIN || EWOULDBLOCK ||
errno == EINTR || errno == ENOSPC ||
errno == ENOBUFS || errno == ENOMEM)) { Ahora está mejor, pero no del todo.
Hay dos razones por las que no nos gusta este diseño. Primero, el error todavía no es muy visible; segundo, debe insertar demasiados espacios para alinear el código.
Es por eso que necesitamos hacer dos mejoras en este estilo de formato. La primera es que necesitamos usar no más de una comparación por línea:hace que los errores sean fáciles de notar. Por ejemplo:
a == 1 && b == 2 && c && d == 3 &&
La segunda mejora es escribir los operadores &&, ||, etc., de una forma más racional, es decir, a la izquierda en lugar de a la derecha.
Mira lo tedioso que es alinear código mediante espacios:
x == a && y == bbbbb && z == cccccccccc &&
Escribir operadores a la izquierda lo hace mucho más rápido y fácil:
x == a && y == bbbbb && z == cccccccccc
El código parece un poco extraño, pero te acostumbrarás muy pronto.
Combinemos estas dos mejoras para escribir nuestro ejemplo de código en el nuevo estilo:
if (!( errno == EAGAIN
|| EWOULDBLOCK
|| errno == EINTR
|| errno == ENOSPC
|| errno == ENOBUFS
|| errno == ENOMEM)) { Sí, ahora es más largo, pero el error también se ha visto claramente.
Estamos de acuerdo en que se ve extraño, pero sin embargo recomendamos esta técnica. Lo hemos estado usando durante medio año y lo disfrutamos mucho.
No nos parece un problema en absoluto que el código se haya alargado. Incluso podríamos escribirlo de esta manera:
const bool error = errno == EAGAIN
|| errno == EWOULDBLOCK
|| errno == EINTR
|| errno == ENOSPC
|| errno == ENOBUFS
|| errno == ENOMEM;
if (!error) { ¿Se siente decepcionado porque el código es demasiado largo y desordenado? Así que hagámoslo una función.
static bool IsInterestingError(int errno)
{
return errno == EAGAIN
|| errno == EWOULDBLOCK
|| errno == EINTR
|| errno == ENOSPC
|| errno == ENOBUFS
|| errno == ENOMEM;
}
....
if (!IsInterestingError(errno)) { Aquí hay otro ejemplo del proyecto WinDjView:
inline bool IsValidChar(int c)
{
return c == 0x9 || 0xA || c == 0xD ||
c >= 0x20 && c <= 0xD7FF ||
c >= 0xE000 && c <= 0xFFFD ||
c >= 0x10000 && c <= 0x10FFFF;
} La función consta de solo unas pocas líneas, pero todavía tiene un error. La función siempre devuelve verdadero . La razón, a la larga, tiene que ver con el formato deficiente y los programadores que mantienen el código durante muchos años y no están dispuestos a leerlo con atención.
Refactoricemos este código en el estilo de "tabla", también agregaría algunos paréntesis:
inline bool IsValidChar(int c)
{
return
c == 0x9
|| 0xA
|| c == 0xD
|| (c >= 0x20 && c <= 0xD7FF)
|| (c >= 0xE000 && c <= 0xFFFD)
|| (c >= 0x10000 && c <= 0x10FFFF);
} No tiene que formatear su código exactamente como le sugerimos. El objetivo de esta publicación es llamar su atención sobre los errores tipográficos en el código escrito "caóticamente". Al organizarlo en el estilo de "tabla", puede evitar muchos errores tipográficos tontos, y eso ya es genial. Así que esperamos que esta publicación te ayude.
Nota
Siendo completamente honestos, debemos advertirle que el formato de "tabla" a veces puede causar daño. Mira este ejemplo:
inline
void elxLuminocity(const PixelRGBi& iPixel,
LuminanceCell< PixelRGBi >& oCell)
{
oCell._luminance = 2220*iPixel._red +
7067*iPixel._blue +
0713*iPixel._green;
oCell._pixel = iPixel;
} Está tomado del proyecto eLynx SDK. El programador quería alinear el código, por lo que agregó 0 antes del valor 713. Desafortunadamente, olvidó que 0 es el primer dígito de un número, lo que significa que este número es octal.
Una matriz de cadenas
Esperamos que la idea sobre el formato de la tabla del código sea clara, pero tenemos ganas de dar un par de ejemplos más. Echemos un vistazo a un caso más. Al traerlo aquí, estamos diciendo que el formato de la tabla debe usarse no solo con condiciones, sino también con otras construcciones diversas de un lenguaje.
El fragmento está tomado del proyecto Asterisk. El error se detecta mediante el siguiente diagnóstico:V653 Se utiliza una cadena sospechosa que consta de dos partes para la inicialización del arreglo. Es posible que falte una coma. Considere inspeccionar este literal:"KW_INCLUDES" "KW_JUMP".
static char *token_equivs1[] =
{
....
"KW_IF",
"KW_IGNOREPAT",
"KW_INCLUDES"
"KW_JUMP",
"KW_MACRO",
"KW_PATTERN",
....
}; Aquí hay un error tipográfico:se olvidó una coma. Como resultado, dos cadenas que tienen un significado completamente diferente se combinan en una, es decir, en realidad tenemos:
.... "KW_INCLUDESKW_JUMP", ....
El error podría evitarse si el programador utilizara el formato de tabla. Entonces, si se omite la coma, será fácil de detectar.
static char *token_equivs1[] =
{
....
"KW_IF" ,
"KW_IGNOREPAT" ,
"KW_INCLUDES" ,
"KW_JUMP" ,
"KW_MACRO" ,
"KW_PATTERN" ,
....
}; Al igual que la última vez, atención, que si ponemos el delimitador a la derecha (una coma en este caso), hay que añadir muchos espacios, lo cual es un inconveniente. Es especialmente inconveniente si hay una nueva línea/frase larga:tendremos que reformatear toda la tabla.
Es por eso que nuevamente recomendamos formatear la tabla de la siguiente manera:
static char *token_equivs1[] =
{
....
, "KW_IF"
, "KW_IGNOREPAT"
, "KW_INCLUDES"
, "KW_JUMP"
, "KW_MACRO"
, "KW_PATTERN"
....
}; Ahora es muy fácil detectar una coma faltante y no hay necesidad de usar muchos espacios:el código es hermoso e intuitivo. Tal vez esta forma de formato pueda parecer inusual, pero te acostumbras rápidamente; pruébalo tú mismo.
Finalmente, aquí está nuestro breve lema. Por regla general, un código bonito suele ser un código correcto.
Escrito por Andrey Karpov.
Este error se encontró con PVS-Studio herramienta de análisis estático.