Various and wrong logical ops used in zdoom c++ code
Posted: Fri Oct 24, 2014 3:37 pm
1)
If you remember this, there was a buggy 'thing->flags6 && MF6_STEPMISSILE' in p_map.cpp, which was fixed in r3578:
sadly, there's another piece of code which contains the same sequence, in the same file and was added in the same commit as the other:
2)r3562 introduced "flags && FFCF_3DRESTRICT" in p_maps.cpp:
3)Apparently since forever, in c_cvars.cpp, there was a bad attempt to exclude non-hexadecimal characters from CVAR_GUID parsing:
well, all that condition is 'false' because no 'x' satisfies both "x < '0'" and "x > '9'", hence apparently you could pass bad characters.
Assuming that the real intention is excluding non-hexadecimal characters, it should be changed to:
These were found thanks to gcc -Wlogical-op.
If you remember this, there was a buggy 'thing->flags6 && MF6_STEPMISSILE' in p_map.cpp, which was fixed in r3578:
Code: Select all
- else if ((thing->flags & MF_MISSILE)&& !(thing->flags6 && MF6_STEPMISSILE) && tm.floorz > thing->z)
- { // [RH] Don't let normal missiles climb steps
+ if (tm.floorz-thing->z > thing->MaxStepHeight)
+ { // too big a step up
Code: Select all
else if ((thing->flags & MF_MISSILE) && !(thing->flags6 && MF6_STEPMISSILE) && tm.floorz > newz)
{ // [RH] Don't let normal missiles climb steps
return false;
}
Code: Select all
f.z || (!(flags && FFCF_3DRESTRICT) && (tmf.thing != NULL && ff_bottom < tmf.z && ff_top < tmf.z + tmf.thing->MaxStepHeight)))
{
tmf.dropoffz = tmf.floorz = ff_top;
tmf.floorpic = *rover->top.texture;
}
3)Apparently since forever, in c_cvars.cpp, there was a bad attempt to exclude non-hexadecimal characters from CVAR_GUID parsing:
Code: Select all
if (value[i] < '0' && value[i] > '9' &&
value[i] < 'A' && value[i] > 'F' &&
value[i] < 'a' && value[i] > 'f')
{
goodv = false;
}
Assuming that the real intention is excluding non-hexadecimal characters, it should be changed to:
Code: Select all
if (value[i] < '0' || (value[i] > '9' &&
value[i] < 'A') || (value[i] > 'F' &&
value[i] < 'a') || value[i] > 'f')
{
goodv = false;
}