Se você está lendo este texto, ou pensou que havia algo errado com a manchete ou viu o nome de um jogo de computador familiar. VVVVVV é um jogo de plataforma independente que roubou os corações de muitos jogadores por sua agradável simplicidade externa e não menos agradável complexidade interna. Alguns dias atrás, o VVVVVV completou 10 anos, e o autor do jogo - Terry Cavanagh - comemorou esse feriado publicando seu código-fonte. Que coisas alucinantes está escondendo? Leia a resposta neste artigo.
1. Introdução
Ah, VVVVVV ... Lembro-me de encontrá-lo logo após o lançamento e como um grande fã de jogos retro com pixel, fiquei muito empolgado em instalá-lo no meu computador. Lembro-me das minhas primeiras impressões: "Isso é tudo? Só correndo pelas salas quadradas? Pensei depois de alguns minutos brincando. Eu não sabia o que estava me esperando na época. Assim que saí do local de partida, me vi em um mundo bidimensional pequeno, mas confuso e florido, cheio de paisagens incomuns e artefatos de pixel desconhecidos para mim.
Eu me empolguei com o jogo. Eventualmente, eu venci completamente o jogo apesar de alguns desafios, como alta complexidade com controle de jogo aplicado com habilidade, por exemplo - o personagem principal não pode pular, mas é capaz de inverter a direção do vetor de gravidade para si mesmo. Não tenho ideia de quantas vezes meu personagem morreu, mas tenho certeza de que o número de mortes é medido em dezenas de centenas. Afinal, cada jogo tem seu próprio gosto :)
Enfim, vamos voltar ao código fonte, publicado
em homenagem ao aniversário do jogo .
No momento, sou desenvolvedor do PVS-Studio, que é um analisador de código estático para C, C ++, C # e Java. Além de desenvolver diretamente, também estamos envolvidos em nossa promoção de produtos. Para nós, uma das melhores maneiras de fazer isso é escrever artigos sobre a verificação de projetos de código aberto. Nossos leitores obtêm artigos interessantes sobre tópicos de programação e temos a oportunidade de demonstrar os recursos do PVS-Studio. Então, quando soube da abertura do código-fonte VVVVVV, simplesmente não consegui superar isso.
Neste artigo, examinaremos alguns erros interessantes encontrados pelo analisador PVS-Studio no código VVVVVV e examinaremos detalhadamente esses erros. Aponte o vetor de gravidade para baixo e fique à vontade - estamos prestes a começar!
Visão geral dos avisos do analisador
Aviso 1
V512 Uma chamada da função 'sprintf' levará ao estouro do buffer '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 você pode ver, as strings
fileSearch e
oldDirectory têm o mesmo tamanho: 260 caracteres. Após escrever o conteúdo da cadeia
oldDirectory na cadeia de formatação (o terceiro argumento
sprintf ), será semelhante a:
<i>contents_oldDirectory\*.vvvvvv</i>
Essa linha tem 9 caracteres a mais que o valor original de
oldDirectory . É essa sequência de caracteres que será gravada em
fileSearch . O que acontece se o comprimento da cadeia
oldDirectory for maior que 251? A sequência resultante será maior que o
fileSearch poderia conter, o que levará a violar os limites da matriz. Quais dados na RAM podem ser danificados e que resultado levarão a ser uma questão retórica :)
Aviso 2
V519 A variável 'background' recebe valores duas vezes sucessivamente. Talvez isso seja um erro. Verifique as linhas: 1367, 1373. Map.cpp 1373
void mapclass::loadlevel(....) { .... case 4:
A mesma variável recebe um valor duas vezes seguidas. No entanto, essa variável não é usada em nenhum lugar entre as atribuições. O que é estranho ... Essa sequência pode não violar a lógica do programa, mas essas atribuições indicam alguma confusão ao escrever o código. Se isso é realmente um erro - apenas o autor poderá dizer com certeza. Embora haja exemplos mais vívidos desse erro no código:
void Game::loadquick(....) { .... else if (pKey == "frames") { frames = atoi(pText); frames = 0; } .... }
Nesse caso, é claro que um erro está oculto em algum lugar na lógica ou na atribuição redundante. Talvez a segunda linha tenha sido escrita temporariamente para depuração e, em seguida, tenha sido esquecida. No total, o PVS-Studio emitiu 8 avisos sobre esses casos.
Aviso 3
O objeto
V808 'pKey' do tipo 'basic_string' foi criado, mas não foi utilizado. 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());
Este código é muito estranho. O analisador alerta sobre a variável criada, mas não utilizada,
pKey , mas, na realidade, o problema foi mais interessante. Intencionalmente, destaquei a linha que acionou o aviso com uma seta, pois essa função contém mais de uma definição de string com o nome
pKey . É isso mesmo, outra variável desse tipo é declarada dentro do loop
for . Ele se sobrepõe ao declarado fora do loop.
Portanto, se você se referir ao valor da string
pKey fora do loop
for , obterá o valor igual a
pElem-> Value () , mas ao fazer o mesmo dentro do loop, obterá o valor igual a
edEntityEl -> Valor () . Sobrepor nomes é um erro bastante grosseiro, que pode ser muito difícil de encontrar sozinho durante a revisão do código.
Aviso 4
V805 desempenho reduzido. É ineficiente identificar uma string vazia usando a construção 'strlen (str)> 0'. Uma maneira mais eficiente é 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; }
O analisador encontrou um fragmento para uma potencial otimização. Ele usa a função
strlen para verificar se a string está vazia. Essa função percorre todos os elementos da string e verifica cada um deles por um terminador nulo ('\ 0'). Se obtivermos uma cadeia longa, cada caractere será comparado com um terminador nulo.
Mas precisamos apenas verificar se a string está vazia! Tudo que você precisa fazer é descobrir se o primeiro caractere de seqüência de caracteres é um terminal nulo. Portanto, para otimizar essa verificação dentro da declaração, vale a pena escrever:
str[0] != '\0'
Essa é a recomendação que o analisador nos dá. Certamente, a chamada da função strlen está na condição da macro
assert , portanto, ela será executada apenas na versão de depuração, onde a velocidade não é tão importante. Na versão de lançamento, a chamada da função e o código serão executados rapidamente. Apesar disso, queria demonstrar o que nosso analisador pode sugerir em termos de micro-otimizações.
Aviso 5
Para demonstrar a essência de outro erro, tenho que citar dois fragmentos de código aqui: a declaração da classe
entclass e seu construtor. Vamos começar com a declaração:
class entclass { public: entclass(); void clear(); bool outside(); public:
Esse construtor de classe tem a seguinte aparência:
entclass::entclass() { clear(); } void entclass::clear() {
Muitos campos, você não diria? Não é de admirar, o PVS-Studio emitiu um aviso para um bug, oculto aqui:
V730 É possível que nem todos os membros de uma classe sejam inicializados dentro do construtor. Considere inspecionar: oldxp, oldyp. Ent.cpp 3
Como você pode ver, duas inicializações de campos de classe foram perdidas em uma lista tão longa. Como resultado, seus valores permaneceram indefinidos, para que possam ser lidos e usados incorretamente em outro local do programa. É muito difícil detectar esse erro apenas analisando.
Aviso 6
Veja este código:
void mapclass::loadlevel(....) { .... std::vector<std::string> tmap; .... tmap = otherlevel.loadlevel(rx, ry, game, obj); fillcontent(tmap); ....
Aviso do PVS-Studio:
V688 A variável local 'tmap' possui o mesmo nome que um dos membros da classe, o que pode resultar em confusão. Map.cpp 1192
De fato, olhando dentro da classe
mapclass , você pode encontrar o mesmo vetor com o mesmo nome:
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;
Infelizmente, a mesma declaração de vetor de nome dentro da função torna o vetor declarado na classe invisível. Acontece que o vetor
tmap é alterado apenas dentro da função
loadlevel . O vetor declarado na classe permanece o mesmo!
Curiosamente, o PVS-Studio encontrou 20 desses fragmentos de código! Na maioria das vezes, eles se referem a variáveis temporárias que foram declaradas "por conveniência" como membros da classe. O autor do jogo (e seu único desenvolvedor) escreveu sobre si mesmo que costumava ter esse mau hábito. Você pode ler sobre isso no post - o link é fornecido no início do artigo.
Ele também observou que esses nomes levavam a erros prejudiciais que eram difíceis de detectar. Bem, esses erros podem ser realmente destrutivos, mas capturá-los se torna menos difícil se você usar a análise estática :)
Aviso 7
V601 O tipo inteiro é convertido implicitamente no tipo char. Game.cpp 4997
void Game::loadquick(....) { .... else if (pKey == "totalflips") { totalflips = atoi(pText); } else if (pKey == "hardestroom") { hardestroom = atoi(pText);
Para entender o que está acontecendo, vamos dar uma olhada nas definições das variáveis da parte do código:
As variáveis
totalflips e
hardestroomdeaths são inteiros; portanto, é perfeitamente normal atribuir a eles o resultado da função
atoi . Mas o que acontece se você atribuir um valor inteiro a
std :: string ? Essa tarefa acaba sendo válida da perspectiva da linguagem. Como resultado, um valor pouco claro será gravado na variável mais
difícil !
Aviso 8
V1004 O ponteiro 'pElem' foi usado sem segurança após ter sido verificado no nullptr. Verifique as linhas: 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();
O analisador avisa que o ponteiro
pElem é usado sem segurança logo após a verificação de
nullptr . Para garantir que o analisador esteja correto, vamos verificar a definição da função
Element () que retorna o valor que, por sua vez, inicializa o
ponteiro pElem:
TiXmlElement *Element() const { return ToElement(); }
Como podemos ver no comentário, essa função pode retornar
nula .
Agora imagine que isso realmente aconteceu. O que acontecerá neste caso? O fato é que essa situação não será tratada de forma alguma. Sim, haverá uma mensagem de que algo deu errado, mas o ponteiro incorreto será desreferenciado apenas uma linha abaixo. Essa desreferenciação resultará em falha do programa ou comportamento indefinido. Este é um erro muito sério.
Aviso 9
Esse fragmento de código acionou quatro avisos do analisador PVS-Studio:
- V560 Uma parte da expressão condicional é sempre verdadeira: x> = 0. editor.cpp 1137
- V560 Uma parte da expressão condicional é sempre verdadeira: y> = 0. editor.cpp 1137
- V560 Uma parte da expressão condicional é sempre verdadeira: x <40. editor.cpp 1137
- V560 Uma parte da expressão condicional é sempre verdadeira: 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; }
Todos os avisos estão relacionados à última declaração
if . O problema é que todas as quatro verificações, realizadas nela, sempre retornam
verdadeiras . Eu não diria que é um erro grave, mas é bem engraçado. O autor decidiu levar essa função a sério e, caso verifique novamente cada variável :)
Ele poderia ter removido essa verificação, pois o fluxo de execução não chegará à expressão "
return 0; " de qualquer maneira. Não mudará a lógica do programa, mas ajudará a livrar-se de verificações redundantes e código morto.
Aviso 10
Em seu artigo no aniversário do jogo, Terry observou ironicamente que um dos elementos que controlavam a lógica do jogo era a grande mudança da função
Game :: updatestate () , responsável por um grande número de estados diferentes do jogo ao mesmo tempo. . E era de se esperar que eu encontrasse o seguinte aviso:
V2008 Complexidade ciclomática: 548. Considere refatorar a função 'Game :: updatestate'. Game.cpp 612
Sim, você acertou: o PVS-Studio atribuiu à função a seguinte classificação de complexidade - 548. Quinhentos e quarenta e oito !!! É assim que o "código legal" se parece. E isso apesar do fato de que, exceto pela instrução switch, quase não há mais nada na função. No próprio switch, contei mais de 300 expressões de caso.
Você sabe, em nossa empresa, temos uma pequena concorrência pelo artigo mais longo. Eu adoraria trazer todo o código de função (3.450 linhas) aqui, mas tal vitória seria injusta, então vou me limitar ao
link para o comutador gigante. Eu recomendo que você siga o link e veja o seu comprimento! Por outro lado, além do
Game :: updatestate () , o PVS-Studio também encontrou 44 funções com complexidade ciclomática inflada, 10 das quais com um número de complexidade superior a 200.
Conclusão
Eu acho que os erros acima são suficientes para este artigo. Sim, houve muitos erros no projeto, mas é um tipo de recurso. Ao abrir seu código, Terry Cavanagh mostrou que você não precisa ser um programador perfeito para escrever um ótimo jogo. Agora, 10 anos depois, Terry relembra esses tempos com ironia. É importante aprender com seus erros, e praticar é a melhor maneira de fazê-lo. E se a sua prática pode dar origem a um jogo como VVVVVV, é simplesmente magnífico! Bem ... Está na hora de jogar mais uma vez :)
Estes não foram todos os erros encontrados no código do jogo. Se você quiser ver por si mesmo o que mais pode ser encontrado - sugiro que você
baixe e experimente o PVS-Studio ! Além disso, não esqueça que
fornecemos projetos de código aberto com licenças gratuitas.