Se você ler este texto, significa que você pensou que havia algo errado com o título do artigo ou viu o nome de um jogo de computador familiar nele. VVVVVV é um jogo de plataforma independente que conquistou os corações de muitos jogadores com sua agradável simplicidade externa e complexidade interna igualmente agradável. Alguns dias atrás, o VVVVVV completou 10 anos e o autor do jogo - Terry Cavanagh - comemorou esse feriado com a publicação do seu código-fonte. O que é "saboroso" pode ser encontrado nele? Leia a resposta neste artigo.
1. Introdução
Ah, VVVVVV ... Lembro-me de como o encontrei logo após o lançamento e, sendo um grande fã de jogos retro com pixel, instalei-o no meu computador. Lembro-me das minhas primeiras impressões: “E então, é tudo? Apenas correr pelas salas quadradas? Pensei depois de alguns minutos do jogo. Ainda não sabia o que me esperava. Assim que saí do local de partida, me vi em um mundo bidimensional pequeno, mas confuso e ornamentado, cheio de paisagens incomuns e artefatos de pixel desconhecidos para mim.
Este jogo me arrastou. Apesar da alta complexidade, habilmente derrotada pelo controle incomum da época (o personagem principal não sabe pular, mas é capaz de inverter a direção do vetor de gravidade para mim), passei por ele completamente. Não sei quantas vezes meu personagem morreu, mas tenho certeza de que o número de mortes é medido em dezenas de centenas. Ainda assim, há um charme único neste jogo :)
Vamos voltar ao código fonte
apresentado em homenagem ao aniversário do jogo .
No momento, sou C ++ - o desenvolvedor da equipe PVS-Studio - um analisador de código estático para C, C ++, C # e Java. Além do próprio desenvolvimento, também estamos envolvidos na promoção de nosso produto. Para nós, uma das melhores maneiras de fazer isso é escrever artigos sobre a verificação de projetos de código aberto. Nossos leitores recebem artigos interessantes sobre tópicos de programação e temos a oportunidade de demonstrar claramente os recursos do PVS-Studio. Portanto, quando soube da abertura do código-fonte VVVVVV, simplesmente não consegui passar.
Neste artigo, examinaremos alguns erros interessantes encontrados pelo analisador PVS-Studio no código VVVVVV, além de realizar uma análise detalhada desses erros. Retorne o vetor de gravidade para a posição “para baixo” e sente-se: estamos começando!
Visão geral dos alertas emitidos pelo 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
linhas fileSearch e
oldDirectory têm o mesmo tamanho: 260 caracteres. A sequência de formatação (o terceiro argumento do
sprintf ) após a substituição do conteúdo da sequência
oldDirectory terá a seguinte aparência:
<i>_oldDirectory\*.vvvvvv</i>
Essa cadeia tem 9 caracteres a mais que o
oldDirectory original. É essa sequência de caracteres que será gravada posteriormente em
fileSearch . O que acontece se a cadeia
oldDirectory for maior que 251? A sequência resultante será maior que o
fileSearch pode acomodar, o que levará à gravação fora da matriz. Que tipo de dados na RAM pode ser danificado e que resultado isso levará a uma pergunta 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:
Uma e a mesma variável recebe um valor duas vezes seguidas. Além disso, entre atribuições, essa variável não é usada em nenhum lugar. Estranho ... Talvez essa sequência não viole a lógica do programa, mas essas atribuições por si só falam de alguma confusão ao escrever código. Se isso é realmente um erro, só pode ser dito pelo autor. Embora haja exemplos mais brilhantes desse erro no código:
void Game::loadquick(....) { .... else if (pKey == "frames") { frames = atoi(pText); frames = 0; } .... }
Já está claro aqui que em algum lugar aqui existe um erro na lógica ou pelo menos uma atribuição desnecessária. Talvez a segunda linha tenha sido escrita temporariamente para depuração e eles se esqueceram de excluí-la. No total, o PVS-Studio emitiu 8 avisos sobre tais situações.
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());
Código muito estranho. O analisador alerta sobre a variável
pKey criada, mas não usada, mas, na verdade, o problema acabou sendo mais interessante. Marquei especificamente com uma seta a linha na qual o aviso foi emitido, porque esta função contém mais de uma definição de linha com o nome
pKey . Sim, outra variável desse tipo é declarada dentro do loop
for e, por seu nome, sobrepõe-se à declarada fora do loop.
Portanto, se você
observar o valor da string
pKey fora do loop
for , obterá um valor igual a
pElem-> Value () , mas se fizer o mesmo dentro do loop, obterá um valor igual a
edEntityEl-> Value () . Sobrepor nomes é um erro bastante grave, que pode ser muito difícil de encontrar por conta própria durante a revisão de 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 local para potencial microoptimização. Ele usa a função
strlen para verificar se há uma string no estilo C. Esta função "percorre" todos os elementos da string e verifica a igualdade de cada um deles com o terminal zero ('\ 0'). Se uma string grande for inserida, cada um de seus caracteres ainda será comparado com uma string zero.
Mas precisamos apenas verificar se a string não está vazia! Para fazer isso, basta descobrir se o primeiro caractere da string é um terminal zero. Portanto, para otimizar essa verificação dentro de assert, você deve escrever:
str[0] != '\0'
Essa é a recomendação que o analisador nos dá. Obviamente, a chamada da função strlen está na condição de macro
assert , por isso 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 de função desaparecerá e o código funcionará rapidamente. No entanto, eu queria demonstrar os recursos do analisador ao propor microoptimizações.
Aviso 5
Para mostrar o seguinte erro, preciso anexar dois
pedaços de código aqui: a declaração da classe
entclass e seu construtor. Vamos começar com o anúncio:
class entclass { public: entclass(); void clear(); bool outside(); public:
O construtor desta classe se parece com isso:
entclass::entclass() { clear(); } void entclass::clear() {
Campos suficientes, não é? Não é de surpreender que ocorra um erro aqui, para o qual o PVS-Studio emitiu um aviso:
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, em uma lista tão grande, a inicialização de dois campos da classe foi perdida. Assim, seus valores permaneceram indefinidos, pelo que, em algum outro lugar do programa, um valor desconhecido pode ser lido e usado a partir deles. É muito difícil detectar esse erro através dos olhos.
Aviso 6
Considere o código:
void mapclass::loadlevel(....) { .... std::vector<std::string> tmap; .... tmap = otherlevel.loadlevel(rx, ry, game, obj); fillcontent(tmap); ....
Aviso 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, se você olhar dentro da classe
mapclass , poderá encontrar lá 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, declarar um vetor com o mesmo nome dentro de uma função torna o vetor declarado na classe invisível. Acontece que, em toda a função de nível de
carga, o vetor
tmap muda apenas dentro da função. O vetor declarado na classe permanece inalterado!
Curiosamente, o PVS-Studio descobriu até 20 desses fragmentos de código! Na maioria das vezes, eles estão associados a variáveis temporárias, que, por conveniência, foram declaradas como membros da classe. O próprio autor do jogo (e seu único desenvolvedor) escreveu que costumava ter esse mau hábito. Você pode ler sobre isso em um post ao qual vinculei no início deste artigo.
Ele também observa que tais nomes levaram a erros prejudiciais e difíceis de capturar. Bem, esses erros podem realmente ser prejudiciais, mas usar a análise estática para capturá-los não será difícil :)
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 qual é o problema, vamos dar uma olhada nas definições de variáveis da seção de código fornecida:
As variáveis
totalflips e
hardestroomdeaths são do tipo inteiro e, portanto, atribuir o resultado à função
atoi nelas é completamente normal. Mas o que acontece se você atribuir um valor inteiro a
std :: string ? Acontece que, do ponto de vista do idioma, essa tarefa é completamente válida. Como resultado, algum valor incompreensível 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 de maneira insegura imediatamente após a verificação de
nullptr . Para garantir que o analisador esteja correto, dê uma olhada na definição da função
Element () , cujo valor de retorno inicializa o ponteiro
pElem :
TiXmlElement *Element() const { return ToElement(); }
Como você pode 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, uma mensagem será exibida informando que algo deu errado, mas literalmente uma linha abaixo do ponteiro incorreto será desreferenciada. Essa desreferenciação, por sua vez, levará a uma falha no programa ou a um comportamento indefinido. Este é um erro muito sério.
Aviso 9
Na próxima seção do código, o PVS-Studio emitiu quatro avisos:
- 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 se aplicam à última declaração
if . O problema é que todas as quatro verificações realizadas nele sempre retornam
verdadeiras . Eu não diria que este é um erro grave, mas acabou sendo muito engraçado. O autor decidiu levar essa função a sério e, por precaução, verificou cada variável novamente :)
Essa verificação pode ser removida porque o encadeamento de execução nunca alcançará a expressão "
return 0; " de qualquer maneira. Embora isso não mude a lógica do programa, ele o libertará de verificações e códigos mortos desnecessários.
Aviso 10
Em seu artigo no aniversário do jogo, Terry diz com ironia que um dos elementos que controlam a lógica do jogo foi uma grande mudança da função
Game :: updatestate () , responsável imediatamente por um grande número de estados diferentes do jogo. 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ê entendeu corretamente: o PVS-Studio estimou a complexidade ciclomática de uma função em 548 unidades. Quinhentos e quarenta e oito !!! Isso eu entendo - "código puro". E isso apesar do fato de que, de fato, não há nada além de uma expressão de comutação em uma função. No próprio switch, contei mais de 300 expressões de caso.
Em nosso escritório, há uma pequena competição entre os autores pelo artigo mais longo. Gostaria de trazer todo o código de função aqui (3450 linhas), mas essa vitória seria desonesta, por isso vou me limitar a simplesmente me
referir à grande mudança. Eu recomendo segui-lo e avaliar a escala inteira! A propósito, além de
Game :: updatestate () , o PVS-Studio encontrou até 44 funções com complexidade ciclomática excessiva, 10 das quais com mais de 200.
Conclusão
Eu acho que os erros escritos são suficientes para o artigo. Sim, houve muitos erros no projeto, mas esse é exatamente o truque: depois de definir seu código, Terry Cavanagh mostrou que não é necessário ser um bom programador para fazer um bom jogo. Agora, depois de 10 anos, Terry relembra esses tempos com ironia. Aprender com seus erros é importante e a prática é a melhor maneira de fazer isso. E se sua prática ainda pode dar origem a um jogo como VVVVVVV, então isso geralmente é maravilhoso! Ehh ... eu vou e provavelmente vou tocar de novo :)
Esses não foram todos os erros encontrados no código do jogo. Se você quiser ver por si mesmo o que mais pode encontrar, sugiro
fazer o download e experimentar o PVS-Studio ! Também não se esqueça que, para projetos de código aberto,
fornecemos uma licença gratuita.
Se você deseja compartilhar este artigo com um público que fala inglês, use o link para a tradução: George Gribkov.
VVVVVV ??? VVVVVV !!! :) .