Lo invitamos a tratar de encontrar un error en una función muy simple del proyecto GNU Midnight Commander. Por qué Solo asi. Es gracioso e interesante. Aunque no, mentimos. Una vez más, queremos demostrar un error que una persona encuentra con dificultad en el proceso de revisión de código, pero encuentra fácilmente el analizador de código estático PVS-Studio.
Recientemente nos enviaron una carta preguntando por qué el analizador genera una advertencia sobre la función
EatWhitespace , cuyo código se proporciona a continuación. En realidad, la pregunta no es tan simple. Intenta descubrir qué está mal con este código tú mismo.
static int EatWhitespace (FILE * InFile) { int c; for (c = getc (InFile); isspace (c) && ('\n' != c); c = getc (InFile)) ; return (c); }
Como puede ver, la función
EatWhitespace es muy pequeña. Incluso un comentario sobre una función ocupa más espacio que el cuerpo de la función en sí :). Ahora algunos detalles.
Descripción de la función
Getc :
int getc ( FILE * stream );
La función devuelve el carácter señalado por el indicador interno de la posición del archivo de la secuencia especificada. Luego el indicador pasa al siguiente carácter. Si se alcanza el final del archivo en el momento de la llamada a la secuencia, la función devuelve
EOF y establece el indicador de final de archivo para esta secuencia. Si se produce un error de lectura, la función devuelve un valor EOF y establece un indicador de error para la secuencia dada (ferror).
Descripción de la función
isspace :
int isspace( int ch );
La función verifica si el carácter es un espacio en blanco de acuerdo con la clasificación del entorno local actual. En la configuración regional estándar, los siguientes caracteres son espacios en blanco:
- espacio (0x20, ``);
- cambio de página (0x0c, '\ f');
- avance de línea LF (0x0a, '\ n');
- retorno de carro CR (0x0d, '\ r');
- pestaña horizontal (0x09, '\ t');
- pestaña vertical (0x0b, '\ v').
Valor de retorno Valor distinto de cero, si el carácter es un espacio en blanco, cero en caso contrario.
La función
EatWhitespace debe omitir todos los caracteres considerados espacios en blanco, excepto el avance de línea '\ n'. Otra razón para detener la lectura de un archivo puede estar llegando al final del archivo (EOF).
Y ahora, sabiendo todo esto, ¡trata de encontrar un error!
Para evitar que el lector accidentalmente no mire inmediatamente la respuesta, agregue un par de unicornios en espera.
Figura 1. Tiempo para buscar un error. Los unicornios esperarán.¿Aún no ves el error?
El caso es que engañamos a los lectores sobre
isspace . Jaja Esta no es una característica estándar en absoluto, sino una macro casera. Sí, somos irreprensibles y te confundimos.
Figura 2. Un unicornio da a los lectores una falsa impresión de lo que es el espacio .En realidad, por supuesto, nosotros y nuestro unicornio no tenemos la culpa. Los autores del proyecto GNU Midnight Commander contribuyeron a la confusión al decidir crear su propia implementación de
isspace en el archivo
charset.h :
#ifdef isspace #undef isspace #endif .... #define isspace(c) ((c)==' ' || (c) == '\t')
Al crear tal macro, algunos desarrolladores confundieron a otros desarrolladores. El código se escribe asumiendo que
isspace es una función estándar que considera los retornos de carro (0x0d, '\ r') como uno de los caracteres de espacio en blanco.
La macro implementada considera solo los espacios y las pestañas como caracteres de espacio en blanco. Sustituyamos la macro y veamos qué sucede.
for (c = getc (InFile); ((c)==' ' || (c) == '\t') && ('\n' != c); c = getc (InFile))
La subexpresión ('\ n'! = C) es redundante (redundante) ya que su resultado siempre será verdadero. El analizador PVS-Studio advierte sobre esto, dando una advertencia:
V560 Una parte de la expresión condicional siempre es verdadera: ('\ n'! = C). params.c 136.
Para mayor claridad, analicemos 3 opciones para el desarrollo de eventos:
- Se alcanza el final del archivo. El final del archivo (EOF) no es un espacio o pestaña. La subexpresión ('\ n'! = C) no se calcula debido a la evaluación de cortocircuito . El ciclo se detiene.
- Se lee cualquier carácter que no sea un espacio o tabulación. La subexpresión ('\ n'! = C) no se calcula debido a la evaluación de cortocircuito. El ciclo se detiene.
- Leer un carácter de espacio o pestaña horizontal. La subexpresión ('\ n'! = C) se calcula, pero su resultado siempre será verdadero.
En otras palabras, el código revisado es equivalente a esto:
for (c = getc (InFile); c==' ' || c == '\t'; c = getc (InFile))
Descubrimos que el código no funciona según lo previsto. Veamos ahora qué consecuencias tiene esto.
El programador que escribió la llamada
isspace en el cuerpo de la función
EatWhitespace esperaba que se
llamara a una función estándar. Es por eso que agregó la condición de que el avance de línea LF ('\ n') no debe considerarse un carácter de espacio en blanco.
Por lo tanto, el programador planeó que además del espacio y las pestañas horizontales, se omitirían caracteres como el cambio de página y la pestaña vertical.
Es de destacar que también se planeó omitir el carácter de retorno de carro CR (0x0d, '\ r'). Esto no sucede y el ciclo se detiene cuando encuentra este símbolo. Esto conducirá a sorpresas desagradables si el separador de línea en el archivo es la secuencia CR + LF utilizada en algunos sistemas que no son UNIX, como Microsoft Windows.
Para aquellos que quieran aprender más sobre las razones históricas para usar LF o CR + LF como separadores de línea, aquí está el artículo de Wikipedia "
Avance de línea ".
Se
supone que la función
EatWhitespace procesa los archivos de la misma manera, donde LF y CR + LF se usan como separadores. Para el caso de CR + LF, esto no es así. En otras palabras, si su archivo proviene del mundo de Windows, entonces no tiene suerte :).
Quizás esto no sea un error grave, especialmente porque GNU Midnight Commander es común en sistemas operativos tipo UNIX, donde el carácter LF (0x0a, '\ n') se usa para traducir una línea. Sin embargo, debido a tales pequeñeces, surgen varios problemas molestos de incompatibilidad de datos preparados en sistemas Linux y Windows.
El error descrito es interesante porque es casi imposible de detectar con una revisión de código clásica. No todos los desarrolladores de proyectos pueden conocer las complejidades de la macro, y olvidarlos es muy fácil. Este es un buen ejemplo donde el análisis de código estático complementa las revisiones de código y otras técnicas de búsqueda de errores.
Anular las funciones estándar es una mala práctica. Por cierto, recientemente en el artículo "
Análisis de código estático de amor " se consideró un caso similar con la macro
#define sprintf std :: printf .
Una mejor solución sería darle a la macro un nombre único, por ejemplo,
is_space_or_tab . Entonces la confusión sería imposible.
Quizás la razón para crear la macro fue la operación lenta de la función estándar de
espacio, y el programador creó una versión más rápida, suficiente para resolver todas las tareas necesarias. Pero aún así, esta decisión es incorrecta. Sería más
confiable definir
isspace de tal manera que obtenga código sin compilar. Y para implementar la funcionalidad necesaria en una macro con un nombre único.
Gracias por su atencion Lo invitamos a
descargar y probar el analizador PVS-Studio para probar sus proyectos. Además, le recordamos que recientemente el analizador ha agregado soporte para el lenguaje Java.
Si desea compartir este artículo con una audiencia de habla inglesa, utilice el enlace a la traducción: Andrey Karpov.
¿Quieres jugar a un detective? Encuentra el error en una función de Midnight Commander .