Antes del lanzamiento de Amnesia: Rebirth, Fractional Games ha publicado el código fuente del legendario Amnesia: The Dark Descent y su secuela, Amnesia: A Machine For Pigs. ¿Por qué no utilizar una herramienta de análisis estático para ver qué terribles errores se esconden en el interior de estos juegos de terror de culto?
Al ver en Reddit la noticia de que se había publicado el código fuente de los juegos " Amnesia: The Dark Descend " y " Amnesia: A Machine for Pigs ", no pude pasar y verificar este código usando PVS-Studio, y al mismo tiempo escribir acerca de este artículo. Sobre todo teniendo en cuenta el hecho de que el 20 de octubre (y en el momento de la publicación de este artículo estaba igualado) una nueva parte de esta serie: " Amnesia: Rebirth ".
Amnesia: The Dark Descent se lanzó en 2010 y se ha convertido en un juego de terror de supervivencia icónico. Honestamente, nunca he podido pasarlo, ni siquiera un poquito, ya que juego juegos de terror de acuerdo con el mismo algoritmo: apostar, ingresar cinco minutos, salir por "alt + f4" en el primer momento de creep y eliminar el juego.Pero me gustó ver el pasaje de este juego en YouTube.
Y en caso de que alguien aún no esté familiarizado con PVS-Studio, este es un analizador estático que busca errores y lugares sospechosos en el código fuente de los programas.
En especial, me gusta mirar el código fuente de los juegos, así que si te preguntas qué errores se cometen en él, puedes leer mis artículos anteriores. Bueno, o echa un vistazo a los artículos de mis compañeros sobre cómo comprobar el código fuente de los juegos.
Después de comprobarlo, resultó que gran parte del código de "The Dark Descend" y "A Machine For Pigs" se superponían, y los informes de los dos proyectos eran muy similares. Entonces, casi todos los errores que citaré a continuación están contenidos en ambos proyectos.
Y la capa más grande de errores de todos los analizadores detectados en estos proyectos fue la capa de errores de "copiar y pegar". De ahí el título del artículo. La principal razón de estos errores es el " efecto de última línea ".
Bueno, vayamos al grano.
Errores de copiar y pegar
Había muchos lugares sospechosos similares a la copia desatendida. Algunos casos, quizás, son causados de alguna manera por la lógica interna del juego en sí. Pero si nos avergonzaron tanto a mí como al analizador, entonces valió la pena al menos hacer un comentario sobre ellos. Después de todo, otros desarrolladores pueden ser tan lentos como yo.
Snippet 1.
Comencemos con un ejemplo donde toda la función consiste en comparar los resultados de métodos y campos de dos objetos aObjectDataA y aObjectDataB . Daré esta función por completo para mayor claridad. Intente notar por sí mismo dónde se cometió el error en la función:
static bool SortStaticSubMeshesForBodies(const ....& aObjectDataA,
const ....& aObjectDataB)
{
//Is shadow caster check
if( aObjectDataA.mpObject->GetRenderFlagBit(....)
!= aObjectDataB.mpObject->GetRenderFlagBit(....))
{
return aObjectDataA.mpObject->GetRenderFlagBit(....)
< aObjectDataB.mpObject->GetRenderFlagBit(....);
}
//Material check
if( aObjectDataA.mpPhysicsMaterial != aObjectDataB.mpPhysicsMaterial)
{
return aObjectDataA.mpPhysicsMaterial < aObjectDataB.mpPhysicsMaterial;
}
//Char collider or not
if( aObjectDataA.mbCharCollider != aObjectDataB.mbCharCollider)
{
return aObjectDataA.mbCharCollider < aObjectDataB.mbCharCollider;
}
return aObjectDataA.mpObject->GetVertexBuffer()
< aObjectDataA.mpObject->GetVertexBuffer();
}Una imagen, para no espiar accidentalmente la respuesta:
¿Podrías encontrar el error? Sí, la última devolución es una comparación utilizando aObjectDataA en ambos lados. Tenga en cuenta que todas las expresiones en el código original se escribieron en una línea, aquí las he dividido para que todo encaje exactamente en el ancho de la línea. Imagínese lo que buscará tal defecto al final de la jornada laboral. Y el analizador lo encontrará inmediatamente después de ensamblar y ejecutar el análisis incremental.
V501 Hay subexpresiones idénticas 'aObjectDataA.mpObject-> GetVertexBuffer ()' a la izquierda y a la derecha del operador '<'. WorldLoaderHplMap.cpp 1123
Como resultado, se encontrará un error similar casi en el momento de escribir el código, en lugar de esconderse en las profundidades del código desde varias etapas de QA, haciendo su búsqueda muchas veces más difícil.
Nota del colega Andrey Karpov. Sí, este es un clásico "error de última línea". Sin embargo, este también es un patrón de error clásico al comparar dos objetos. Consulte el artículo " Vidas malvadas en funciones de comparación ".Fragmento 2.
Echemos un vistazo al código que provocó la advertencia:
Presento el código con una captura de pantalla para mayor claridad.
La advertencia en sí misma se ve así:
V501 Hay sub-expresiones idénticas 'lType == eLuxJournalState_OpenNote' a la izquierda y a la derecha de '||' operador. LuxJournal.cpp 2262
El analizador ha detectado que hay un error al verificar el valor de la variable lType . La igualdad se comprueba dos veces con el mismo miembro del enumerador eLuxJournalState_OpenNote .
Primero, me gustaría que esta condición se escribiera en forma de "tabla" para mejorar la legibilidad. Consulte el capítulo N13 del mini-libro The Biggest Question of Programming, Refactoring, and All That .
if(!( lType == eLuxJournalState_OpenNote
|| lType == eLuxJournalState_OpenDiary
|| lType == eLuxJournalState_OpenNote
|| lType == eLuxJournalState_OpenNarratedDiary))
return false;De esta forma, resulta mucho más fácil detectar un error incluso sin un analizador.
Sin embargo, surge la pregunta, ¿una verificación tan errónea conduce a una distorsión de la lógica del programa? Después de todo, es posible que se deba verificar algún otro valor de lType , pero la verificación se perdió debido a un error de copiar y pegar. Echemos un vistazo a la enumeración en sí:
enum eLuxJournalState
{
eLuxJournalState_Main,
eLuxJournalState_Notes,
eLuxJournalState_Diaries,
eLuxJournalState_QuestLog,
eLuxJournalState_OpenNote,
eLuxJournalState_OpenDiary,
eLuxJournalState_OpenNarratedDiary,
eLuxJournalState_LastEnum,
};Solo hay tres significados que tienen la palabra "Abrir" en su nombre. Y los tres están presentes en el cheque. Lo más probable es que aquí no haya distorsión de la lógica, pero aún es imposible decirlo con certeza. De modo que el analizador encontró un error lógico que el desarrollador del juego podía corregir o encontró un lugar escrito "feo" que debería haber sido reescrito para mayor claridad.
Fragmento 3.
El siguiente caso es generalmente el ejemplo más obvio de un error de copiar y pegar.
V778 Se encontraron dos fragmentos de código similares. Quizás, esto es un error tipográfico y la variable 'mvSearcherIDs' debería usarse en lugar de 'mvAttackerIDs'. LuxSavedGameTypes.cpp 615
void cLuxMusicHandler_SaveData::ToMusicHandler(....)
{
....
// Enemies
//Attackers
for(size_t i=0; i<mvAttackerIDs.Size(); ++i)
{
iLuxEntity *pEntity = apMap
->GetEntityByID(mvAttackerIDs[i]);
if(....)
{
....
}
else
{
Warning("....", mvAttackerIDs[i]);
}
}
//Searchers
for(size_t i=0; i<mvSearcherIDs.Size(); ++i)
{
iLuxEntity *pEntity = apMap->GetEntityByID(mvSearcherIDs[i]);
if(....)
{
....
}
else
{
Warning("....", mvAttackerIDs[i]);
}
}
}En el primer ciclo, el trabajo va con el puntero pEntity , que se recibió a través de mvAttackerID, y si no se cumple la condición, se emite un mensaje de depuración para los mismos mvAttackerID . Sin embargo, en el siguiente ciclo, que tiene exactamente el mismo estilo que la sección de código anterior, pEntity se obtiene usando mvSearcherIDs . Pero la advertencia todavía se emite con la mención de mvAttackerIDs .
Lo más probable es que el bloque de código "Buscadores" se haya copiado del bloque "Atacantes", mvAttackerIDs se reemplazó por mvSearcherIDs , pero el bloque else no se modificó. Como resultado, el mensaje de error utiliza un elemento de la matriz incorrecta.
Este error no afecta la lógica del juego, pero de esta manera puedes ponerle un cerdo serio a una persona que tendrá que depurar este lugar y perder tiempo trabajando con el elemento mvSearcherIDs incorrecto .
Fragmento 4.
El analizador indicó el siguiente lugar sospechoso con tres advertencias:
- V547 La expresión 'pEntity == 0' siempre es falsa. LuxScriptHandler.cpp 2444
- V649 Hay dos sentencias 'if' con expresiones condicionales idénticas. La primera instrucción 'if' contiene la función return. Esto significa que la segunda declaración "si" no tiene sentido. Verifique las líneas: 2433, 2444. LuxScriptHandler.cpp 2444
- V1051 Considere la posibilidad de buscar errores de impresión. Es posible que la 'pTargetEntity' deba estar marcada aquí. LuxScriptHandler.cpp 2444
Veamos el código:
void __stdcall cLuxScriptHandler::PlaceEntityAtEntity(....)
{
cLuxMap *pMap = gpBase->mpMapHandler->GetCurrentMap();
iLuxEntity *pEntity = GetEntity(....);
if(pEntity == NULL) return;
if(pEntity->GetBodyNum() == 0)
{
....
}
iPhysicsBody *pBody = GetBodyInEntity(....);
if(pBody == NULL) return;
iLuxEntity *pTargetEntity = GetEntity(....);
if(pEntity == NULL) return; // <=
iPhysicsBody *pTargetBody = GetBodyInEntity(....);
if(pTargetBody == NULL) return;
....
}Se emitió una advertencia V547 para la segunda verificación pEntity == NULL . Para el analizador, esta verificación siempre será falsa , ya que si esta condición fuera verdadera , entonces la salida de la función se habría producido en mayor medida debido a la verificación similar anterior.
La siguiente advertencia, V649 , se emitió precisamente porque tenemos dos controles idénticos. Por lo general, este caso puede no ser un error. De repente, una parte del código implementa una lógica, y en otra parte del código, de acuerdo con la misma verificación, se debe implementar algo más. Pero en este caso, el cuerpo del primer cheque consiste en return, por lo que la segunda verificación, si la condición resulta ser cierta, ni siquiera llegará al punto. Al rastrear dicha lógica, el analizador reduce la cantidad de mensajes falsos para códigos sospechosos y los envía solo para una lógica muy extraña.
El error indicado por la última advertencia es inherentemente muy similar al ejemplo anterior. Lo más probable es que todas las comprobaciones se hayan duplicado desde la primera comprobación si (pEntity == NULL) , y luego el objeto comprobado se reemplazó por el requerido. En el caso de los objetos pBody y pTargetBody, se realizó el reemplazo, pero se olvidó el objeto pTargetEntity . Como resultado, este objeto no se comprueba.
En el ejemplo que estamos considerando, si profundiza un poco más en el código, resulta que tal error en general no afectará el curso del programa. El puntero pTargetBody obtiene su valor de la función GetBodyInEntity :
iPhysicsBody *pTargetBody = GetBodyInEntity(pTargetEntity,
asTargetBodyName);El primer argumento aquí es solo un puntero no verificado previamente, que no se usa en ningún otro lugar. Y afortunadamente, dentro de esta función hay una verificación del primer argumento para NULL :
iPhysicsBody* ....::GetBodyInEntity(iLuxEntity* apEntity, ....)
{
if(apEntity == NULL){
return NULL;
}
....
}Como resultado, este código funciona correctamente al final, aunque contiene un error.
Fragmento 5. ¡
Y un lugar más sospechoso con copiar y pegar!
Este método borra los campos del objeto de la clase cLuxPlayer .
void cLuxPlayer::Reset()
{
....
mfRoll=0;
mfRollGoal=0;
mfRollSpeedMul=0; //<-
mfRollMaxSpeed=0; //<-
mfLeanRoll=0;
mfLeanRollGoal=0;
mfLeanRollSpeedMul=0;
mfLeanRollMaxSpeed=0;
mvCamAnimPos =0;
mvCamAnimPosGoal=0;
mfRollSpeedMul=0; //<-
mfRollMaxSpeed=0; //<-
....
}Pero por alguna razón, las dos variables mfRollSpeedMul y mfRollMaxSpeed se anulan dos veces:
- V519 A la variable 'mfRollSpeedMul' se le asignan valores dos veces seguidas. Quizás esto sea un error. Verifique las líneas: 298, 308. LuxPlayer.cpp 308
- V519 A la variable 'mfRollMaxSpeed' se le asignan valores dos veces seguidas. Quizás esto sea un error. Líneas de control: 299, 309. LuxPlayer.cpp 309
Veamos la clase en sí y veamos qué campos contiene:
class cLuxPlayer : ....
{
....
private:
....
float mfRoll;
float mfRollGoal;
float mfRollSpeedMul;
float mfRollMaxSpeed;
float mfLeanRoll;
float mfLeanRollGoal;
float mfLeanRollSpeedMul;
float mfLeanRollMaxSpeed;
cVector3f mvCamAnimPos;
cVector3f mvCamAnimPosGoal;
float mfCamAnimPosSpeedMul;
float mfCamAnimPosMaxSpeed;
....
}Curiosamente, hay tres bloques similares de variables con nombres relacionados: mfRoll , mfLeanRoll y mvCamAnimPos . En Reset, estos tres bloques se borran, excepto las dos últimas variables del tercer bloque, mfCamAnimPosSpeedMul y mfCamAnimPosMaxSpeed . Y es exactamente en el lugar de estas dos variables donde se encuentran asignaciones duplicadas. Lo más probable es que todas estas asignaciones se copiaron del primer bloque de asignación y luego los nombres de las variables se reemplazaron por los requeridos.
Puede ser que dos variables faltantes no deberían haberse puesto a cero, pero también es muy probable lo contrario. Y las reasignaciones obviamente no ayudarán a mantener este código. Como puede ver, en un paño largo de las mismas acciones, es posible que no note tal error, y el analizador ayuda aquí.
Fragmento 5.5.
Este ejemplo es muy similar al anterior. Le daré un fragmento de código y una advertencia del analizador.
V519 A la variable 'mfTimePos' se le asignan valores dos veces seguidas. Quizás esto sea un error. Verifique las líneas: 49, 53. AnimationState.cpp 53
cAnimationState::cAnimationState(....)
{
....
mfTimePos = 0;
mfWeight = 1;
mfSpeed = 1.0f;
mfBaseSpeed = 1.0f;
mfTimePos = 0;
mfPrevTimePos=0;
....
}A la variable mfTimePos se le ha asignado dos veces el valor 0. Como en el ejemplo anterior, veamos la declaración de este campo:
class cAnimationState
{
....
private:
....
//Properties of the animation
float mfLength;
float mfWeight;
float mfSpeed;
float mfTimePos;
float mfPrevTimePos;
....
}Puede ver que este bloque de declaración también coincide con el orden de asignación en el fragmento de código de error, como en el ejemplo anterior. Aquí, en la asignación, en lugar de la variable mfLength, el valor obtiene mfTimePos . Pero aquí el error no se puede explicar copiando el bloque y el "efecto de la última línea". Es posible que no sea necesario asignar un nuevo valor a mfLength , pero esta ubicación sigue siendo sospechosa.
Fragmento 6.
El analizador emitió un montón de advertencias para el siguiente fragmento de código de "Amnesia: A Machine For Pigs". Daré solo una parte del código para el cual se emitieron errores del mismo tipo:
void cLuxEnemyMover::UpdateMoveAnimation(float afTimeStep)
{
....
if(prevMoveState != mMoveState)
{
....
//Backward
if(mMoveState == eLuxEnemyMoveState_Backward)
{
....
}
....
//Walking
else if(mMoveState == eLuxEnemyMoveState_Walking)
{
bool bSync = prevMoveState == eLuxEnemyMoveState_Running
|| eLuxEnemyMoveState_Jogging
? true : false;
....
}
....
}
}¿Dónde está el error aquí?
El analizador emitió las siguientes advertencias:
- V768 La constante de enumeración 'eLuxEnemyMoveState_Jogging' se utiliza como una variable de tipo booleano. LuxEnemyMover.cpp 672
- V768 La constante de enumeración 'eLuxEnemyMoveState_Walking' se utiliza como una variable de tipo booleano. LuxEnemyMover.cpp 680
- V768 La constante de enumeración 'eLuxEnemyMoveState_Jogging' se utiliza como una variable de tipo booleano. LuxEnemyMover.cpp 688
La cadena if-else-if se repite en el código original, y luego estas advertencias se emitieron para cada cuerpo de cada otro if .
Considere la línea apuntada por el analizador:
bool bSync = prevMoveState == eLuxEnemyMoveState_Running
|| eLuxEnemyMoveState_Jogging
? true : false;No es de extrañar que un error se haya infiltrado en una expresión de este tipo, que también está escrita en una línea en el original. Y probablemente ya lo hayas notado. El miembro de la enumeración eLuxEnemyMoveState_Jogging no se compara con nada; se comprueba su valor. Lo más probable es que la expresión 'prevMoveState == eLuxEnemyMoveState_Jogging' estuviera implícita.
Tal error puede parecer completamente inofensivo. Pero en mi otro artículo, sobre la verificación del motor Bullet , entre las confirmaciones del proyecto, encontré una corrección de erroresdel mismo tipo, lo que provoca que se apliquen fuerzas a los objetos desde el lado equivocado. Y en este caso, este error se cometió varias veces. Bueno, también noto que la condición ternaria es completamente sin sentido, ya que se cumplirá, en último lugar, con los resultados booleanos de los operadores lógicos.
Fragmento 7.
Y, finalmente, el último par de ejemplos de errores de copiar y pegar. Esta vez de nuevo en una declaración condicional. El analizador emitió una advertencia para el siguiente código:
void iParticleEmitter::SetSubDivUV(const cVector2l &avSubDiv)
{
//Check so that there is any subdivision
// and that no sub divison axis is
//equal or below zero
if( (avSubDiv.x > 1 || avSubDiv.x > 1) && (avSubDiv.x >0 && avSubDiv.y >0))
{
....
}
....
}Creo que es bastante fácil notar un lugar sospechoso en un fragmento de este tipo, separado del código completo. Sin embargo, este error logró esconderse de los desarrolladores de este juego.
El analizador emitió la siguiente advertencia:
V501 Hay subexpresiones idénticas a la izquierda y a la derecha de '||' operador: avSubDiv.x> 1 || avSubDiv.x> 1 ParticleEmitter.cpp 199
El segundo paréntesis de la condición muestra que los campos x e y están marcados . Pero en el primer paréntesis, por alguna razón se perdió este momento y solo se marca el campo x... Además, a juzgar por el comentario de validación, ambos campos deberían haber sido validados. Aquí de alguna manera no fue el "efecto de la última línea" lo que funcionó, sino más bien el primero, porque en el primer paréntesis se olvidaron de reemplazar la llamada al campo x con la llamada al campo y .
Entonces, tales errores son bastante insidiosos, ya que en este caso el desarrollador ni siquiera fue ayudado escribiendo un comentario explicativo de la condición.
En tales casos, recomendaría tomar como hábito registrar los cheques relacionados en forma tabular. Es más fácil editar de esta manera y la falla será mucho más notoria:
if( (avSubDiv.x > 1 || avSubDiv.x > 1)
&& (avSubDiv.x > 0 && avSubDiv.y > 0))Fragmento 7.5.
Absolutamente igual, de hecho, el error se encontró en un lugar completamente diferente:
static bool EdgeTriEqual(const cTriEdge &edge1, const cTriEdge &edge2)
{
if(edge1.tri1 == edge2.tri1 && edge1.tri2 == edge2.tri2)
return true;
if(edge1.tri1 == edge1.tri1 && edge1.tri2 == edge2.tri1)
return true;
return false;
}Bueno, ¿cómo te las arreglaste para ver inmediatamente dónde se escondía? No en vano se han resuelto tantos ejemplos :)
El analizador emitió la siguiente advertencia:
V501 Hay sub-expresiones idénticas a la izquierda y a la derecha del operador '==': edge1.tri1 == edge1.tri1 Math.cpp 2914
Analicemos esto fragmento en orden. Obviamente, la primera comprobación comprueba la igualdad de los campos edge1.tri1 y edge2.tri2 , y al mismo tiempo la igualdad de edge1.tri2 y edge2.tri2 :
edge1.tri1 -> edge2.tri1
edge1.tri2 -> edge2.tri2Y en la segunda verificación, a juzgar por la parte correcta de la verificación 'edge1.tri2 == edge2.tri1', fue necesario verificar la igualdad de estos campos "en forma transversal":
Pero en lugar de verificar edge1.tri1 == edge2.tri2 , se realizó una verificación sin sentido edge1.tri1 == edge1.tri1 . Pero este es todo el contenido de la función, no eliminé nada de ella. Y de todos modos, tal error entró en el código.
Otros errores
Fragmento 1.
Daré el siguiente fragmento de código con sangrías originales.
void iCharacterBody::CheckMoveCollision(....)
{
....
/////////////////////////////////////
//Forward velocity reflection
//Make sure that new velocity points in the right direction
//and that it is not too large!
if(mfMoveSpeed[eCharDir_Forward] != 0)
{
vForwardVel = ....;
float fForwardSpeed = vForwardVel.Length();
if(mfMoveSpeed[eCharDir_Forward] > 0)
if(mfMoveSpeed[eCharDir_Forward] > fForwardSpeed)
mfMoveSpeed[eCharDir_Forward] = fForwardSpeed;
else
if(mfMoveSpeed[eCharDir_Forward] < fForwardSpeed)
mfMoveSpeed[eCharDir_Forward] = -fForwardSpeed;
}
....
}Advertencia de PVS-Studio: V563 Es posible que esta rama 'else' deba aplicarse a la declaración 'if' anterior. CharacterBody.cpp 1591
Este ejemplo puede resultar confuso. ¿Por qué más tiene la misma sangría que el si exterior ? ¿Es lo demás para la condición externa? Pues bien, es necesario colocar los paréntesis, si no otra cosa se refiere a la que conduce directamente si .
if(mfMoveSpeed[eCharDir_Forward] > 0)
{
if(mfMoveSpeed[eCharDir_Forward] > fForwardSpeed)
mfMoveSpeed[eCharDir_Forward] = fForwardSpeed;
}
else if(mfMoveSpeed[eCharDir_Forward] < fForwardSpeed)
{
mfMoveSpeed[eCharDir_Forward] = -fForwardSpeed;
}¿O no? En el proceso de redacción de este artículo, cambié de opinión varias veces sobre qué variante de la secuencia de las acciones concebidas para este código es más probable.
Si profundizamos un poco más en este código, resulta que la variable fForwardSpeed , con la que se realiza la comparación en el if inferior , no puede tener un valor menor que cero, ya que recibe un valor del método Length :
inline T Length() const
{
return sqrt( x * x + y * y + z * z);
}Entonces, lo más probable es que la esencia de estas comprobaciones sea que primero comprobemos si el elemento mfMoveSpeed es mayor que cero, y luego comprobamos su valor relativo a fForwardSpeed . Además, los dos últimos si se corresponden entre sí en su redacción.
En este caso, el código original funcionará según lo previsto. Pero definitivamente hará que la persona que venga a editarlo / refactorizarlo se convierta en un rompecabezas.
Pensé que nunca me encontraría con un código con este aspecto. Por interés, busqué en nuestra base de datos de errores encontrados en proyectos de código abierto y descritos en artículos. Y se encontraron ejemplos de este error en otros proyectos; puede verlos usted mismo .
Y por favor no escribas así, aunque tú mismo lo tengas claro en este caso. O llaves rizadas o sangría correcta, o mejor, ambas. No se sumerja en el sufrimiento de quienes lleguen a comprender su código, y de usted mismo en el futuro;)
Fragmento 2.
El siguiente error me confundió un poco, y traté de encontrar la lógica aquí durante mucho tiempo. Pero al final, todavía me parece que esto es probablemente un error, y bastante grave.
Echemos un vistazo al código:
bool cBinaryBuffer::DecompressAndAdd(char *apSrcData, size_t alSize)
{
....
///////////////////////////
// Init decompression
int ret = inflateInit(&zipStream);
if (ret != Z_OK) return false;
///////////////////////////
// Decompress, chunk by chunk
do
{
//Set current output chunk
zipStream.avail_out = lMaxChunkSize;
....
//Decompress as much as possible to current chunk
int ret = inflate(&zipStream, Z_NO_FLUSH);
if(ret != Z_OK && ret != Z_STREAM_END)
{
inflateEnd(&zipStream);
return false;
}
....
}
while (zipStream.avail_out == 0 && ret != Z_STREAM_END);
....
return true;
}V711 Es peligroso crear una variable local dentro de un bucle con el mismo nombre que una variable que controla este bucle. BinaryBuffer.cpp 371
Entonces, tenemos una variable ret , que controla la salida del ciclo do-while . Pero dentro de este ciclo, en lugar de asignar un nuevo valor a esta variable externa, se declara una nueva variable llamada ret . Como resultado, anula la variable externa ret y la variable que está marcada en la condición del ciclo nunca cambiará.
En una lamentable combinación de circunstancias, tal ciclo podría volverse infinito. Lo más probable es que, en este caso, este código guarde una condición interna que verifique el valor de la variable interna ret y conduce a salir de la función.
Conclusión
Muy a menudo, los desarrolladores utilizan el análisis estático no con regularidad, sino con descansos prolongados. O incluso ejecutan el proyecto a través del analizador una vez. Como resultado de este enfoque, el analizador a menudo no detecta nada serio o encuentra algo como los ejemplos que estamos considerando, lo que quizás no afectó particularmente la funcionalidad del juego. Parece que el analizador no es particularmente útil. Bueno, encontró esos lugares, pero aún funciona.
Y es que lugares similares, pero en los que el error no estaba enmascarado, pero claramente condujo a un error en el programa, ya han sido corregidos por largas horas de depuración, ejecuciones de pruebas y el departamento de pruebas. Como resultado, al verificar un proyecto, el analizador muestra solo aquellos problemas que no se han manifestado de ninguna manera. A veces, entre tales problemas también hay momentos serios que realmente influyeron en el funcionamiento del programa, pero la probabilidad de que el programa siguiera su camino era baja, por lo que los desarrolladores desconocían este error.
Por lo tanto, es extremadamente importante evaluar la utilidad del análisis estático solo después de su uso regular. Si una única ejecución a través de PVS-Studio reveló lugares tan sospechosos y descuidados en el código de este juego, entonces, ¿cuántos errores obvios de este tipo tuvieron que ser localizados y corregidos en el proceso de desarrollo?
¡Utilice un analizador estático con regularidad!
Si desea compartir este artículo con una audiencia de habla inglesa, utilice el enlace de traducción: Victoria Khanieva. Amnesia: The Dark Descent o Cómo olvidar arreglar Copiar Pegar .