Si lees este texto, significa que pensaste que algo andaba mal con el título del artículo o que viste el nombre de un juego de computadora familiar en él. VVVVVV es un juego de plataformas independiente que se ha ganado los corazones de muchos jugadores con su simplicidad externa agradable y su complejidad interna igualmente agradable. Hace unos días, VVVVVV cumplió 10 años, y el autor del juego, Terry Cavanagh, celebró estas vacaciones con la publicación de su código fuente. ¿Qué es "sabroso" se puede encontrar en él? Lee la respuesta en este artículo.
Introduccion
Oh, VVVVVV ... Recuerdo cómo lo encontré poco después del lanzamiento y, como soy un gran fanático de los juegos retro de píxeles, felizmente lo instalé en mi computadora. Recuerdo mis primeras impresiones: “¿Y eso es todo? ¿Simplemente correr por las habitaciones cuadradas? Pensé después de unos minutos del juego. Todavía no sabía lo que me esperaba. Tan pronto como salí de la ubicación inicial, me encontré en un mundo bidimensional pequeño pero confuso y adornado lleno de paisajes inusuales y artefactos de píxeles desconocidos para mí.
Este juego me arrastró. A pesar de la alta complejidad, hábilmente vencido por el control inusual en ese momento (el personaje principal no sabe cómo saltar, pero es capaz de invertir la dirección del vector de gravedad por mí mismo), lo pasé por completo. No tengo idea de cuántas veces murió mi personaje, pero estoy seguro de que el número de muertes se mide en decenas de cientos. Aún así, hay un encanto único en este juego :)
Volvamos al código fuente
presentado en honor del aniversario del juego .
Por el momento, soy C ++, un desarrollador del equipo PVS-Studio, un analizador de código estático para C, C ++, C # y Java. Además del desarrollo en sí, también nos dedicamos a la promoción de nuestro producto. Para nosotros, una de las mejores maneras de hacerlo es escribir artículos sobre la verificación de proyectos de código abierto. Nuestros lectores reciben artículos interesantes sobre temas de programación, y tenemos la oportunidad de demostrar claramente las capacidades de PVS-Studio. Por lo tanto, cuando me enteré de la apertura del código fuente VVVVVV, simplemente no pude pasar.
En este artículo analizaremos algunos errores interesantes encontrados por el analizador PVS-Studio en el código VVVVVV, y realizaremos un análisis detallado de estos errores. Regrese el vector de gravedad a la posición "abajo" y siéntese: ¡estamos comenzando!
Resumen de alertas emitidas por el analizador
Advertencia 1
V512 Una llamada de la función 'sprintf' provocará el desbordamiento del búfer 'fileSearch'. FileSystemUtils.cpp 307
#define MAX_PATH 260 .... void PLATFORM_migrateSaveData(char *output) { char oldLocation[MAX_PATH]; char newLocation[MAX_PATH]; char oldDirectory[MAX_PATH]; char fileSearch[MAX_PATH]; .... strcpy(oldDirectory, output); sprintf(fileSearch, "%s\\*.vvvvvv", oldDirectory); .... }
Como puede ver, las
líneas fileSearch y
oldDirectory tienen el mismo tamaño: 260 caracteres. La cadena de formato (el tercer argumento para
sprintf ) después de sustituir el contenido de la cadena
oldDirectory en ella se verá así:
<i>_oldDirectory\*.vvvvvv</i>
Esta cadena es 9 caracteres más larga que el antiguo
directorio original. Es esta secuencia de caracteres que se escribirá más en
fileSearch . ¿Qué sucede si la cadena
oldDirectory es más larga que 251? La cadena resultante será más larga de lo que puede admitir
fileSearch , lo que conducirá a escribir fuera de la matriz. ¿Qué tipo de datos en la RAM pueden estar dañados y qué resultado dará lugar a una pregunta retórica :)
Advertencia 2
V519 La variable 'fondo' tiene valores asignados dos veces seguidas. Quizás esto sea un error. Líneas de verificación: 1367, 1373. Map.cpp 1373
void mapclass::loadlevel(....) { .... case 4:
A una misma variable se le asigna un valor dos veces seguidas. Además, entre asignaciones, esta variable no se usa en ningún lado. Extraño ... Quizás esta secuencia no viole la lógica del programa, pero tales tareas en sí mismas hablan de cierta confusión al escribir código. Si esto es realmente un error solo puede decirlo el autor. Aunque hay ejemplos más brillantes de este error en el código:
void Game::loadquick(....) { .... else if (pKey == "frames") { frames = atoi(pText); frames = 0; } .... }
Ya está claro aquí que en algún lugar aquí se encuentra un error en la lógica, o al menos una asignación innecesaria. Quizás la segunda línea se escribió temporalmente para la depuración, y luego se olvidaron de eliminarla. En total, PVS-Studio emitió 8 advertencias sobre tales situaciones.
Advertencia 3
Se creó el objeto V808 'pKey' del tipo 'basic_string' pero no se utilizó. editor.cpp 1866
void editorclass::load(std::string &_path) { .... std::string pKey(pElem->Value()); .... if (pKey == "edEntities") { int i = 0; for (TiXmlElement *edEntityEl = pElem->FirstChildElement(); edEntityEl; edEntityEl = edEntityEl->NextSiblingElement()) { std::string pKey(edEntityEl->Value());
Código muy raro El analizador advierte sobre la variable
pKey creada pero no utilizada, pero de hecho el problema resultó ser más interesante. Marqué específicamente con una flecha la línea en la que se emitió la advertencia, porque esta función contiene más de una definición de línea con el nombre
pKey . Sí, otra variable de este tipo se declara dentro del ciclo
for , y por su nombre se superpone a la declarada fuera del ciclo.
Por lo tanto, si
observa el valor de la cadena
pKey fuera del ciclo
for , obtendrá un valor igual a
pElem-> Value () , pero si hace lo mismo dentro del ciclo, obtendrá un valor igual a
edEntityEl-> Value () . La superposición de nombres es un error bastante grave, que puede ser muy difícil de encontrar por su cuenta durante la revisión del código.
Advertencia 4
V805 Disminución del rendimiento. Es ineficiente identificar una cadena vacía usando la construcción 'strlen (str)> 0'. Una forma más eficiente es verificar: str [0]! = '\ 0'. physfs.c 1604
static char *prefDir = NULL; .... const char *PHYSFS_getPrefDir(const char *org, const char *app) { .... assert(strlen(prefDir) > 0); ... return prefDir; }
El analizador encontró un lugar para la potencial microoptimización. Utiliza la función
strlen para verificar una cadena de estilo C para anular. Esta función "recorre" todos los elementos de la cadena y comprueba la igualdad de cada uno de ellos con el terminal cero ('\ 0'). Si se ingresa una cadena grande, entonces cada uno de sus caracteres se comparará con una cadena cero.
¡Pero solo necesitamos verificar que la cadena no esté vacía! Para hacer esto, solo averigua si el primer carácter de la cadena es un terminal cero. Por lo tanto, para optimizar esta verificación dentro de la afirmación, debe escribir:
str[0] != '\0'
Esta es la recomendación que nos da el analizador. Por supuesto, la llamada a la función strlen está en la condición de macro de aserción, por lo que solo se ejecutará en la versión de depuración, donde la velocidad no es tan importante. En la versión de lanzamiento, la llamada a la función desaparecerá y el código funcionará rápidamente. Sin embargo, quería demostrar las capacidades del analizador al proponer microoptimizaciones.
Advertencia 5
Para mostrar el siguiente error, necesito adjuntar dos
piezas de código aquí: la declaración de la clase
entclass y su constructor. Comencemos con el anuncio:
class entclass { public: entclass(); void clear(); bool outside(); public:
El constructor de esta clase se ve así:
entclass::entclass() { clear(); } void entclass::clear() {
Suficientes campos, ¿no? No es sorprendente que haya un error al acecho, a lo que PVS-Studio emitió una advertencia:
V730 Es posible que no todos los miembros de una clase se inicialicen dentro del constructor. Considere inspeccionar: oldxp, oldyp. Ent.cpp 3
Como puede ver, en una lista tan grande, se perdió la inicialización de dos campos de la clase. Por lo tanto, sus valores permanecieron indefinidos, por lo que, en otro lugar del programa, se puede leer y usar un valor desconocido de ellos. Es muy difícil detectar dicho error a través de los ojos.
Advertencia 6
Considera el código:
void mapclass::loadlevel(....) { .... std::vector<std::string> tmap; .... tmap = otherlevel.loadlevel(rx, ry, game, obj); fillcontent(tmap); ....
Advertencia PVS-studio:
V688 La variable local 'tmap' posee el mismo nombre que uno de los miembros de la clase, lo que puede generar confusión. Map.cpp 1192
De hecho, si miras dentro de la clase
mapclass , puedes encontrar allí el mismo vector con el mismo nombre:
class mapclass { public: .... std::vector <int> roomdeaths; std::vector <int> roomdeathsfinal; std::vector <int> areamap; std::vector <int> contents; std::vector <int> explored; std::vector <int> vmult; std::vector <std::string> tmap;
Desafortunadamente, declarar un vector con el mismo nombre dentro de una función hace que el vector declarado en la clase sea invisible. Resulta que a lo largo de la función de nivel de
carga, el vector
tmap solo cambia dentro de la función. ¡El vector declarado en la clase permanece sin cambios!
¡Curiosamente, PVS-Studio descubrió hasta 20 fragmentos de código! En su mayor parte, están asociados con variables temporales, que, por conveniencia, se declararon como miembros de la clase. El propio autor del juego (y su único desarrollador) escribió que solía tener este mal hábito. Puedes leer sobre esto en una publicación a la que me vinculé al principio de este artículo.
También señala que tal denominación condujo a errores dañinos y difíciles de atrapar. Bueno, tales errores realmente pueden ser dañinos, pero usar el análisis estático para atraparlos no será difícil :)
Advertencia 7
V601 El tipo entero se
convierte implícitamente en el tipo char. Game.cpp 4997
void Game::loadquick(....) { .... else if (pKey == "totalflips") { totalflips = atoi(pText); } else if (pKey == "hardestroom") { hardestroom = atoi(pText);
Para comprender cuál es el problema, echemos un vistazo a las definiciones de variables de la sección de código dada:
Las variables
totalflips y
hardestroomdeaths son de tipo entero y, por lo tanto, asignar el resultado a la función
atoi en ellas es completamente normal. Pero, ¿qué sucede si asigna un valor entero a
std :: string ? Resulta que desde el punto de vista del lenguaje, tal asignación es completamente válida. Como resultado, ¡algún valor incomprensible se escribirá en la variable
hardestroom !
Advertencia 8
V1004 El puntero 'pElem' se usó de forma insegura después de que se verificó contra nullptr. Líneas de verificación: 1739, 1744. editor.cpp 1744
void editorclass::load(std::string &_path) { .... TiXmlHandle hDoc(&doc); TiXmlElement *pElem; TiXmlHandle hRoot(0); version = 0; { pElem = hDoc.FirstChildElement().Element();
El analizador advierte que el puntero
pElem se usa de
forma insegura inmediatamente después de que se haya verificado para
nullptr . Para asegurarse de que el analizador es correcto, eche un vistazo a la definición de la función
Element () , cuyo valor de retorno inicializa el puntero
pElem :
TiXmlElement *Element() const { return ToElement(); }
Como puede ver en el comentario, esta función puede devolver
nulo .
Ahora imagine que esto realmente sucedió. ¿Qué pasará en este caso? El hecho es que esta situación no se manejará de ninguna manera. Sí, se mostrará un mensaje que indica que algo salió mal, pero literalmente se desreferenciará una línea debajo del puntero incorrecto. Tal desreferenciación, a su vez, provocará un bloqueo del programa o un comportamiento indefinido. Este es un error bastante grave.
Advertencia 9
En la siguiente sección de código, PVS-Studio emitió cuatro advertencias:
- V560 Una parte de la expresión condicional siempre es verdadera: x> = 0. editor.cpp 1137
- V560 Una parte de la expresión condicional siempre es verdadera: y> = 0. editor.cpp 1137
- V560 Una parte de la expresión condicional siempre es verdadera: x <40. Editor.cpp 1137
- V560 Una parte de la expresión condicional siempre es verdadera: y <30. Editor.cpp 1137
int editorclass::at( int x, int y ) { if(x<0) return at(0,y); if(y<0) return at(x,0); if(x>=40) return at(39,y); if(y>=30) return at(x,29); if(x>=0 && y>=0 && x<40 && y<30) { return contents[x+(levx*40)+vmult[y+(levy*30)]]; } return 0; }
Todas las advertencias se aplican a la última declaración
if . El problema es que las cuatro comprobaciones que se realizan siempre devolverán
verdadero . No diría que este es un error grave, pero resultó bastante divertido. El autor decidió tomar esta función en serio y, por si acaso, volvió a verificar cada variable :)
Esta verificación se puede eliminar porque el hilo de ejecución nunca alcanzará la expresión "
return 0; " de todos modos. Aunque esto no cambia la lógica del programa, lo liberará de verificaciones innecesarias y código muerto.
Advertencia 10
En su artículo sobre el aniversario del juego, Terry dice con ironía que uno de los elementos que controlan la lógica del juego fue un gran cambio de la función
Game :: updatestate () , que es responsable inmediatamente de una gran cantidad de estados de juego diferentes. Y era bastante esperado que encontrara la siguiente advertencia:
V2008 Complejidad ciclomática: 548. Considere refactorizar la función 'Game :: updatestate'. Game.cpp 612
Sí, entendiste correctamente: PVS-Studio estimó la complejidad ciclomática de una función en 548 unidades. Quinientos cuarenta y ocho !!! Esto lo entiendo: "código ordenado". Y esto a pesar del hecho de que, de hecho, no hay nada más que un cambio de expresión en una función. En el cambio en sí, conté más de 300 expresiones de casos.
En nuestra oficina hay una pequeña competencia entre los autores por el artículo más largo. Con mucho gusto traería todo el código de función aquí (3450 líneas), pero tal victoria sería deshonesta, por lo que me limitaré a
referirme simplemente al gran cambio. ¡Recomiendo seguirlo y evaluar la escala completa usted mismo! Por cierto, además de
Game :: updatestate () , PVS-Studio encontró hasta 44 funciones con excesiva complejidad ciclomática, 10 de las cuales tienen una complejidad de más de 200.
Conclusión
Creo que los errores escritos son suficientes para el artículo. Sí, hubo muchos errores en el proyecto, pero este es exactamente el truco: después de haber presentado su código, Terry Cavanagh demostró que no es necesario ser un buen programador para hacer un buen juego. Ahora, después de 10 años, Terry recuerda esos momentos con ironía. Aprender de tus errores es importante, y practicar es la mejor manera de hacerlo. Y si tu práctica aún puede dar lugar a un juego como VVVVVVV, ¡esto es generalmente magnífico! Ehh ... iré y probablemente lo vuelva a jugar :)
Estos no fueron todos los errores encontrados en el código del juego. Si quieres ver por ti mismo qué más puedes encontrar, te sugiero que
descargues y pruebes PVS-Studio . Además, no olvide que para proyectos de código abierto
proporcionamos una licencia gratuita.
Si desea compartir este artículo con una audiencia de habla inglesa, utilice el enlace a la traducción: George Gribkov.
VVVVVV ??? VVVVVV !!! :) .