En este artículo, lo invitamos a intentar encontrar un error en una función muy simple del proyecto GNU Midnight Commander. Por qué Por ninguna razón en particular. Solo por diversión. Bueno, está bien, es mentira. En realidad, queríamos mostrarle otro error que un revisor humano tiene dificultades para encontrar y el analizador de código estático PVS-Studio puede detectar sin esfuerzo.
Un usuario nos envió un correo electrónico el otro día, preguntándole por qué estaba recibiendo una advertencia sobre la función
EatWhitespace (vea el código a continuación). Esta pregunta no es tan trivial como podría parecer. Intenta descubrir por ti mismo lo que está mal con este código.
static int EatWhitespace (FILE * InFile) { int c; for (c = getc (InFile); isspace (c) && ('\n' != c); c = getc (InFile)) ; return (c); }
Como puede ver,
EatWhitespace es una pequeña función; su cuerpo es incluso más pequeño que el comentario :) Ahora, revisemos algunos detalles.
Aquí está la descripción de la función
getc :
int getc ( FILE * stream );
Devuelve el carácter al que apunta actualmente el indicador de posición del archivo interno de la secuencia especificada. El indicador de posición del archivo interno avanza al siguiente carácter. Si la secuencia se encuentra al final del archivo cuando se llama, la función devuelve
EOF y establece el indicador de fin de archivo para la secuencia. Si se produce un error de lectura, la función devuelve EOF y establece el indicador de error para la secuencia (ferror).
Y aquí está la descripción de la función
isspace :
int isspace( int ch );
Comprueba si el carácter dado es un carácter de espacio en blanco según la clasificación de la configuración regional C actualmente instalada. En la configuración regional predeterminada, los caracteres de espacio en blanco son los siguientes:
- espacio (0x20 '');
- alimentación de formulario (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 de lo contrario.
Se
espera que la función
EatWhitespace omita todos los caracteres de espacio en blanco excepto el avance de línea '\ n'. La función también dejará de leer el archivo cuando encuentre Fin de archivo (EOF).
Ahora que sabes todo eso, ¡trata de encontrar el error!
Los dos unicornios a continuación se asegurarán de que no mires accidentalmente el comentario.
Figura 1. Tiempo para la búsqueda de errores. Los unicornios están esperando.¿Aún no tienes suerte?
Bueno, ya ves, es porque te hemos mentido sobre el
espacio . Bwa-ha-ha! No es una función estándar, es una macro personalizada. Sí, somos malos y te confundimos.
Figura 2. Unicornio que confunde a los lectores sobre isspace .No somos nosotros o nuestro unicornio los culpables, por supuesto. La culpa de toda la confusión recae en los autores del proyecto GNU Midnight Commander, quienes hicieron su propia implementación de
isspace en el archivo charset.h:
#ifdef isspace #undef isspace #endif .... #define isspace(c) ((c)==' ' || (c) == '\t')
Con esta macro, los autores confundieron a otros desarrolladores. El código se escribió bajo el supuesto de que
isspace es una función estándar, que considera el retorno de carro (0x0d, '\ r') un carácter de espacio en blanco.
La macro personalizada, a su vez, trata solo los caracteres de espacio y tabulación como caracteres de espacio en blanco. Sustituyamos esa macro y veamos qué sucede.
for (c = getc (InFile); ((c)==' ' || (c) == '\t') && ('\n' != c); c = getc (InFile))
La subexpresión ('\ n'! = C) es innecesaria (redundante) ya que siempre se evaluará como verdadera. Eso es lo que PVS-Studio le advierte al enviar la advertencia:
V560 Una parte de la expresión condicional siempre es verdadera: ('\ n'! = C). params.c 136.
Para que quede claro, examinemos 3 resultados posibles:
- Fin del archivo alcanzado. EOF no es un espacio o un carácter de tabulación. La subexpresión ('\ n'! = C) no se evalúa debido a la evaluación de cortocircuito . El ciclo termina.
- La función ha leído algún carácter que no es un espacio o un carácter de tabulación. La subexpresión ('\ n'! = C) no se evalúa debido a la evaluación de cortocircuito. El ciclo termina.
- La función ha leído un espacio o un carácter de tabulación horizontal. Se evalúa la subexpresión ('\ n'! = C), pero su resultado es siempre verdadero.
En otras palabras, el código anterior es equivalente al siguiente:
for (c = getc (InFile); c==' ' || c == '\t'; c = getc (InFile))
Hemos encontrado que no funciona de la manera deseada. Ahora veamos cuáles son las implicaciones.
Un desarrollador, que escribió la llamada de
isspace en el cuerpo de la función
EatWhitespace esperaba que se
llamara a la función estándar. Es por eso que agregaron la condición que impide que el carácter LF ('\ n') sea tratado como un carácter de espacio en blanco.
Significa que, además del espacio y los caracteres de tabulación horizontales, también planeaban omitir el avance de formulario y los caracteres de tabulación vertical.
Lo que es más notable es que también querían omitir el carácter de retorno de carro (0x0d, '\ r'). Sin embargo, no sucede: el ciclo termina cuando se encuentra con este personaje. El programa terminará comportándose inesperadamente si las nuevas líneas están representadas por la secuencia CR + LF, que es el tipo utilizado en algunos sistemas que no son UNIX como Microsoft Windows.
Para obtener más detalles sobre las razones históricas para usar LF o CR + LF como caracteres de nueva línea, consulte la página de Wikipedia "
Nueva línea ".
La función
EatWhitespace estaba destinada a procesar archivos de la misma manera, ya sea que usaran LF o CR + LF como caracteres de nueva línea. Pero falla en el caso de CR + LF. En otras palabras, si su archivo es del mundo de Windows, está en problemas :).
Si bien esto podría no ser un error grave, especialmente teniendo en cuenta que GNU Midnight Commander se usa en sistemas operativos similares a UNIX, donde LF (0x0a, '\ n') se usa como un personaje de nueva línea, las cosas insignificantes aún tienden a ser molestas problemas con la compatibilidad de datos preparados en Linux y Windows.
Lo que hace que este error sea interesante es que es casi seguro que lo pasará por alto mientras realiza una revisión de código estándar. Los detalles específicos de la implementación de la macro son fáciles de olvidar, y algunos autores de proyectos pueden no conocerlos en absoluto. Es un ejemplo muy vívido de cómo el análisis de código estático contribuye a la revisión de código y otras técnicas de detección de errores.
Anular las funciones estándar es una mala práctica. Por cierto, discutimos un caso similar de la macro
#define sprintf std :: printf en el reciente artículo "
Apreciar el análisis de código estático ".
Una mejor solución hubiera sido darle a la macro un nombre único, por ejemplo,
is_space_or_tab . Esto habría ayudado a evitar toda la confusión.
Quizás la función estándar de
isspace era demasiado lenta y el programador creó una versión más rápida, suficiente para sus necesidades. Pero todavía no deberían haberlo hecho así. Una solución más segura sería definir
isspace para que obtenga un código no compilable, mientras que la funcionalidad deseada podría implementarse como una macro con un nombre único.
Gracias por leer No dude en
descargar PVS-Studio y probarlo con sus proyectos. Como recordatorio, ahora también admitimos Java.