maxmustermann/godot
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Various fixes detected using PVS-Studio static analyzer.

Browse source 
- Add FIXME tags comments to some unfixed potential bugs
- Remove some checks (always false: unsigned never < 0)
- Fix some if statements based on reviews.
- Bunch of missing `else` statements
This commit is contained in:
Thaer Razeq 2017-02-23 02:28:09 -06:00 committed by REBELLIOUSX\Rebel_X
parent 0f8c6dd382
commit f50488a361
32 changed files with 41 additions and 39 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
core/io/http_client.cpp
View file

@ -340,7 +340,7 @@ Error HTTPClient::poll(){
int rs = response_str.size();
if (
(rs>=2 && response_str[rs-2]=='\n' && response_str[rs-1]=='\n') ||
(rs>=4 && response_str[rs-4]=='\r' && response_str[rs-3]=='\n' && rs>=4 && response_str[rs-2]=='\r' && response_str[rs-1]=='\n')
(rs>=4 && response_str[rs-4]=='\r' && response_str[rs-3]=='\n' && response_str[rs-2]=='\r' && response_str[rs-1]=='\n')
) {

4
core/io/marshalls.cpp
View file

@ -641,7 +641,6 @@ Error decode_variant(Variant& r_variant,const uint8_t *p_buffer, int p_len,int *
ERR_FAIL_COND_V(len<4,ERR_INVALID_DATA);
uint32_t count = decode_uint32(buf);
ERR_FAIL_COND_V(count<0,ERR_INVALID_DATA);
PoolVector<String> strings;
buf+=4;
@ -691,7 +690,6 @@ Error decode_variant(Variant& r_variant,const uint8_t *p_buffer, int p_len,int *
ERR_FAIL_COND_V(len<4,ERR_INVALID_DATA);
uint32_t count = decode_uint32(buf);
ERR_FAIL_COND_V(count<0,ERR_INVALID_DATA);
buf+=4;
len-=4;
@ -729,7 +727,6 @@ Error decode_variant(Variant& r_variant,const uint8_t *p_buffer, int p_len,int *
ERR_FAIL_COND_V(len<4,ERR_INVALID_DATA);
uint32_t count = decode_uint32(buf);
ERR_FAIL_COND_V(count<0,ERR_INVALID_DATA);
buf+=4;
len-=4;
@ -768,7 +765,6 @@ Error decode_variant(Variant& r_variant,const uint8_t *p_buffer, int p_len,int *
ERR_FAIL_COND_V(len<4,ERR_INVALID_DATA);
uint32_t count = decode_uint32(buf);
ERR_FAIL_COND_V(count<0,ERR_INVALID_DATA);
buf+=4;
len-=4;

2
core/io/stream_peer.cpp
View file

@ -492,6 +492,8 @@ Error StreamPeerBuffer::get_partial_data(uint8_t* p_buffer, int p_bytes,int &r_r
PoolVector<uint8_t>::Read r = data.read();
copymem(p_buffer,r.ptr(),r_received);
// FIXME: return what? OK or ERR_*
}
int StreamPeerBuffer::get_available_bytes() const {

4
core/pool_allocator.cpp
View file

@ -504,7 +504,7 @@ const void *PoolAllocator::get(ID p_mem) const {
return NULL;
}
if (e->pos<0 || (int)e->pos>=pool_size) {
if ((int)e->pos>=pool_size) {
mt_unlock();
ERR_PRINT("e->pos<0 || e->pos>=pool_size");
@ -546,7 +546,7 @@ void *PoolAllocator::get(ID p_mem) {
return NULL;
}
if (e->pos<0 || (int)e->pos>=pool_size) {
if ((int)e->pos>=pool_size) {
mt_unlock();
ERR_PRINT("e->pos<0 || e->pos>=pool_size");

4
core/variant.cpp
View file

@ -2953,7 +2953,7 @@ uint32_t Variant::hash() const {
PoolVector<p_type>::Read rr = r.read(); \
\
for(int i = 0; i < l.size(); ++i) { \
if(! p_compare_func((lr[0]), (rr[0]))) \
if(! p_compare_func((lr[i]), (rr[i]))) \
return false; \
}\
\
@ -3065,7 +3065,7 @@ bool Variant::hash_compare(const Variant& p_variant) const {
return false;
for(int i = 0; i < l.size(); ++i) {
if(! l[0].hash_compare(r[0]))
if(! l[i].hash_compare(r[i]))
return false;
}

4
core/variant_op.cpp
View file

@ -1393,7 +1393,7 @@ void Variant::set(const Variant& p_index, const Variant& p_value, bool *r_valid)
v->basis.set_axis(index,p_value);
return;
}
} if (p_index.get_type()==Variant::STRING) {
} else if (p_index.get_type()==Variant::STRING) {
Transform *v=_data._transform;
const String *str=reinterpret_cast<const String*>(p_index._data._mem);
@ -2150,7 +2150,7 @@ Variant Variant::get(const Variant& p_index, bool *r_valid) const {
valid=true;
return index==3?v->origin:v->basis.get_axis(index);
}
} if (p_index.get_type()==Variant::STRING) {
} else if (p_index.get_type()==Variant::STRING) {
const Transform *v=_data._transform;
const String *str=reinterpret_cast<const String*>(p_index._data._mem);

2
core/variant_parser.cpp
View file

@ -1414,7 +1414,7 @@ Error VariantParser::parse_value(Token& token,Variant &value,Stream *p_stream,in
return OK;
} else if (id=="img") { // compatibility with godot.cfg
Token token;
Token token; // FIXME: no need for this declaration? the first argument in line 509 is a Token& token.
get_token(p_stream,token,line,r_err_str);
if (token.type!=TK_PARENTHESIS_OPEN) {
r_err_str="Expected '(' in old-style godot.cfg construct";

2
drivers/gles3/rasterizer_canvas_gles3.cpp
View file

@ -1010,7 +1010,7 @@ void RasterizerCanvasGLES3::canvas_render_items(Item *p_item_list,int p_z,const
if (unshaded || (state.canvas_item_modulate.a>0.001 && (!shader_cache || shader_cache->canvas_item.light_mode!=RasterizerStorageGLES3::Shader::CanvasItem::LIGHT_MODE_LIGHT_ONLY) && !ci->light_masked ))
_canvas_item_render_commands(ci,current_clip,reclip);
if ((blend_mode==RasterizerStorageGLES3::Shader::CanvasItem::BLEND_MODE_MIX || RasterizerStorageGLES3::Shader::CanvasItem::BLEND_MODE_PMALPHA) && p_light && !unshaded) {
if ((blend_mode==RasterizerStorageGLES3::Shader::CanvasItem::BLEND_MODE_MIX || blend_mode==RasterizerStorageGLES3::Shader::CanvasItem::BLEND_MODE_PMALPHA) && p_light && !unshaded) {
Light *light = p_light;
bool light_used=false;

1
drivers/gles3/rasterizer_scene_gles3.cpp
View file

@ -2806,6 +2806,7 @@ void RasterizerSceneGLES3::_copy_to_front_buffer(Environment *env) {
//no environment, simply convert from linear to srgb
storage->shaders.copy.set_conditional(CopyShaderGLES3::LINEAR_TO_SRGB,true);
} else {
/* Why are both statements equal? */
storage->shaders.copy.set_conditional(CopyShaderGLES3::LINEAR_TO_SRGB,true);
}

1
drivers/gles3/shader_compiler_gles3.cpp
View file

@ -401,6 +401,7 @@ String ShaderCompilerGLES3::_dump_node_code(SL::Node *p_node, int p_level, Gener
String scode = _dump_node_code(bnode->statements[i],p_level,r_gen_code,p_actions,p_default_actions);
if (bnode->statements[i]->type==SL::Node::TYPE_CONTROL_FLOW || bnode->statements[i]->type==SL::Node::TYPE_CONTROL_FLOW) {
// FIXME: if (A || A) ? I am hesitant to delete one of them, could be copy-paste error.
code+=scode; //use directly
} else {
code+=_mktab(p_level)+scode+";\n";

4
main/tests/test_math.cpp
View file

@ -144,11 +144,11 @@ class GetClassAndNamespace {
error_str="Unterminated comment";
error=true;
return TK_ERROR;
} if (code[idx]=='*' &&code[idx+1]=='/') {
} else if (code[idx]=='*' &&code[idx+1]=='/') {
idx+=2;
break;
} if (code[idx]=='\n') {
} else if (code[idx]=='\n') {
line++;
}

2
modules/gdscript/gd_function.cpp
View file

@ -725,7 +725,7 @@ Variant GDFunction::call(GDInstance *p_instance, const Variant **p_args, int p_a
err.argument-=1;
}
}
} if (methodstr=="free") {
} else if (methodstr=="free") {
if (err.error==Variant::CallError::CALL_ERROR_INVALID_METHOD) {

2
modules/regex/regex.cpp
View file

@ -321,7 +321,7 @@ struct RegExNodeClass : public RegExNode {
case Type_lower:
return ('a' <= c && c <= 'z');
case Type_print:
return (0x1F < c && c < 0x1F);
return (0x20 < c && c < 0x7f);
case Type_punct:
return (REGEX_NODE_PUNCT.find(c) >= 0);
case Type_space:

2
modules/visual_script/visual_script_builtin_funcs.cpp
View file

@ -329,7 +329,7 @@ PropertyInfo VisualScriptBuiltinFunc::get_input_value_port_info(int p_idx) const
case LOGIC_CLAMP: {
if (p_idx==0)
return PropertyInfo(Variant::REAL,"a");
else if (p_idx==0)
else if (p_idx==0) // is it ok to test p_idx == 0 twice?
return PropertyInfo(Variant::REAL,"min");
else
return PropertyInfo(Variant::REAL,"max");

1
scene/2d/particles_2d.cpp
View file

@ -229,6 +229,7 @@ ParticleAttractor2D::ParticleAttractor2D() {
/****************************************/
_FORCE_INLINE_ static float _rand_from_seed(uint64_t *seed) {
uint32_t r = Math::rand_from_seed(seed);
return 2.0f * (float)r / (float)Math::RANDOM_MAX - 1.0f;
}

2
scene/gui/control.cpp
View file

@ -2421,8 +2421,6 @@ void Control::get_argument_options(const StringName& p_function,int p_idx,List<S
Theme::get_default()->get_font_list(get_class(),&sn);
} else if (pf=="add_constant_override" || pf=="has_constant" || pf=="has_constant_override" || pf=="get_constant") {
Theme::get_default()->get_constant_list(get_class(),&sn);
} else if (pf=="add_color_override" || pf=="has_color" || pf=="has_color_override" || pf=="get_color") {
Theme::get_default()->get_color_list(get_class(),&sn);
}
sn.sort_custom<StringName::AlphCompare>();

2
scene/gui/dialogs.cpp
View file

@ -201,7 +201,7 @@ void AcceptDialog::_notification(int p_what) {
if (p_what==NOTIFICATION_MODAL_CLOSE) {
cancel_pressed();
} if (p_what==NOTIFICATION_RESIZED) {
} else if (p_what==NOTIFICATION_RESIZED) {
_update_child_rects();
}

2
scene/gui/item_list.cpp
View file

@ -314,7 +314,7 @@ void ItemList::move_item(int p_item,int p_to_pos) {
if (current<0) {
//do none
} if (p_item==current) {
} else if (p_item==current) {
current=p_to_pos;
} else if (p_to_pos>p_item && current>p_item && current<p_to_pos) {
current--;

2
scene/gui/rich_text_label.h
View file

@ -173,7 +173,7 @@ private:
struct ItemNewline : public Item {
int line;
int line; // FIXME: Overriding base's line ?
ItemNewline() { type=ITEM_NEWLINE; }
};

2
scene/gui/text_edit.cpp
View file

@ -4267,7 +4267,7 @@ void TextEdit::_update_completion_candidates() {
//no completion here
//print_line("cancel!");
cancel=true;
} if (inquote && first_quote!=-1) {
} else if (inquote && first_quote!=-1) {
s=l.substr(first_quote,cofs-first_quote);
//print_line("s: 1"+s);

2
scene/resources/default_theme/default_theme.cpp
View file

@ -986,7 +986,7 @@ void make_default_theme(bool p_hidpi,Ref<Font> p_font) {
Ref<BitmapFont> default_font;
if (p_font.is_valid()) {
default_font=p_font;
} if (p_hidpi) {
} else if (p_hidpi) {
default_font=make_font2(_hidpi_font_height,_hidpi_font_ascent,_hidpi_font_charcount,&_hidpi_font_charrects[0][0],_hidpi_font_kerning_pair_count,&_hidpi_font_kerning_pairs[0][0],_hidpi_font_img_width,_hidpi_font_img_height,_hidpi_font_img_data);
} else {
default_font=make_font2(_lodpi_font_height,_lodpi_font_ascent,_lodpi_font_charcount,&_lodpi_font_charrects[0][0],_lodpi_font_kerning_pair_count,&_lodpi_font_kerning_pairs[0][0],_lodpi_font_img_width,_lodpi_font_img_height,_lodpi_font_img_data);

2
servers/physics/shape_sw.cpp
View file

@ -181,7 +181,7 @@ void RayShapeSW::get_supports(const Vector3& p_normal,int p_max,Vector3 *r_suppo
r_amount=2;
r_supports[0]=Vector3(0,0,0);
r_supports[1]=Vector3(0,0,length);
} if (p_normal.z>0) {
} else if (p_normal.z>0) {
r_amount=1;
*r_supports=Vector3(0,0,length);
} else {

4
servers/visual/shader_language.cpp
View file

@ -307,7 +307,7 @@ ShaderLanguage::Token ShaderLanguage::_get_token() {
} if (GETCHAR(0)=='*' && GETCHAR(1)=='/') {
char_idx+=2;
break;
} if (GETCHAR(0)=='\n') {
} else if (GETCHAR(0)=='\n') {
tk_line++;
}
@ -3241,7 +3241,7 @@ Error ShaderLanguage::_parse_shader(const Map< StringName, Map<StringName,DataTy
_set_error("void datatype not allowed here");
return ERR_PARSE_ERROR;
}
if (!uniform && type<TYPE_FLOAT && type>TYPE_VEC4) {
if (!uniform && type<TYPE_FLOAT && type>TYPE_VEC4) { // FIXME: always false! should it be || instead?
_set_error("Invalid type for varying, only float,vec2,vec3,vec4 allowed.");
return ERR_PARSE_ERROR;
}

8
tools/editor/collada/collada.cpp
View file

@ -352,7 +352,7 @@ void Collada::_parse_image(XMLParser& parser) {
image.path=path;
} if (name=="data") {
} else if (name=="data") {
ERR_PRINT("COLLADA Embedded image data not supported!");
@ -728,7 +728,7 @@ void Collada::_parse_effect_material(XMLParser& parser,Effect &effect,String &id
#endif
}
} if (parser.get_node_type() == XMLParser::NODE_ELEMENT_END && (
} else if (parser.get_node_type() == XMLParser::NODE_ELEMENT_END && (
parser.get_node_name()=="constant" ||
parser.get_node_name()=="lambert" ||
parser.get_node_name()=="phong" ||
@ -1100,7 +1100,7 @@ void Collada::_parse_mesh_geometry(XMLParser& parser,String p_id,String p_name)
current_source=id;
COLLADA_PRINT("source data: "+id);
} else if (section=="float_array" || section=="array" || section=="float_array") {
} else if (section=="float_array" || section=="array") {
// create a new array and read it.
if (meshdata.sources.has(current_source)) {
@ -2192,7 +2192,7 @@ void Collada::_parse_scene(XMLParser& parser) {
state.root_visual_scene=_uri_to_id(parser.get_attribute_value("url"));
print_line("***ROOT VISUAL SCENE: "+state.root_visual_scene);
} if (name=="instance_physics_scene") {
} else if (name=="instance_physics_scene") {
state.root_physics_scene=_uri_to_id(parser.get_attribute_value("url"));

2
tools/editor/import/resource_importer_obj.cpp
View file

@ -118,7 +118,7 @@ Error ResourceImporterOBJ::import(const String& p_source_file, const String& p_s
nrm.y=v[2].to_float();
nrm.z=v[3].to_float();
normals.push_back(nrm);
} if (l.begins_with("f ")) {
} else if (l.begins_with("f ")) {
//vertex
has_index_data=true;

2
tools/editor/import/resource_importer_scene.cpp
View file

@ -1279,7 +1279,7 @@ Error ResourceImporterScene::import(const String& p_source_file, const String& p
Ref<EditorScenePostImport> post_import_script;
if (post_import_script_path!="") {
post_import_script_path = post_import_script_path;
post_import_script_path = post_import_script_path; // FIXME: is there a good reason for this?
Ref<Script> scr = ResourceLoader::load(post_import_script_path);
if (!scr.is_valid()) {
EditorNode::add_io_error(TTR("Couldn't load post-import script:")+" "+post_import_script_path);

2
tools/editor/plugins/collision_polygon_2d_editor_plugin.cpp
View file

@ -268,7 +268,7 @@ bool CollisionPolygon2DEditor::forward_gui_input(const InputEvent& p_event) {
return true;
}
}
} if (mb.button_index==BUTTON_RIGHT && mb.pressed && edited_point==-1) {
} else if (mb.button_index==BUTTON_RIGHT && mb.pressed && edited_point==-1) {

2
tools/editor/plugins/light_occluder_2d_editor_plugin.cpp
View file

@ -283,7 +283,7 @@ bool LightOccluder2DEditor::forward_gui_input(const InputEvent& p_event) {
return true;
}
}
} if (mb.button_index==BUTTON_RIGHT && mb.pressed && edited_point==-1) {
} else if (mb.button_index==BUTTON_RIGHT && mb.pressed && edited_point==-1) {

2
tools/editor/plugins/navigation_polygon_editor_plugin.cpp
View file

@ -312,7 +312,7 @@ bool NavigationPolygonEditor::forward_gui_input(const InputEvent& p_event) {
return true;
}
}
} if (mb.button_index==BUTTON_RIGHT && mb.pressed && edited_point==-1) {
} else if (mb.button_index==BUTTON_RIGHT && mb.pressed && edited_point==-1) {
int closest_outline=-1;
int closest_idx=-1;

2
tools/editor/plugins/polygon_2d_editor_plugin.cpp
View file

@ -385,7 +385,7 @@ bool Polygon2DEditor::forward_gui_input(const InputEvent& p_event) {
return true;
}
}
} if (mb.button_index==BUTTON_RIGHT && mb.pressed && edited_point==-1) {
} else if (mb.button_index==BUTTON_RIGHT && mb.pressed && edited_point==-1) {

1
tools/editor/plugins/texture_region_editor_plugin.cpp
View file

@ -353,6 +353,7 @@ void TextureRegionEditor::_region_input(const InputEvent& p_input)
undo_redo->add_undo_method(atlas_tex.ptr(),"set_region",rect_prev);
}
else if(node_patch9){
// FIXME: Is this intentional?
} else if(node_patch9){
undo_redo->add_do_method(node_patch9 ,"set_region_rect",node_patch9->get_region_rect());
undo_redo->add_undo_method(node_patch9,"set_region_rect",rect_prev);

4
tools/editor/property_editor.cpp
View file

@ -534,7 +534,7 @@ bool CustomPropertyEditor::edit(Object* p_owner,const String& p_name,Variant::Ty
} else if (hint==PROPERTY_HINT_TYPE_STRING) {
/* FIXME: This is repeated twice, with slightly different behavior! Which one? Check line 644 */
if (!create_dialog) {
create_dialog = memnew( CreateDialog );
create_dialog->connect("create",this,"_create_dialog_callback");
@ -651,6 +651,7 @@ bool CustomPropertyEditor::edit(Object* p_owner,const String& p_name,Variant::Ty
} else if (hint==PROPERTY_HINT_TYPE_STRING) {
if (!create_dialog) {
/* FIXME: ... and here. See line 529 */
create_dialog = memnew( CreateDialog );
create_dialog->connect("create",this,"_create_dialog_callback");
add_child(create_dialog);
@ -2384,6 +2385,7 @@ void PropertyEditor::set_item_text(TreeItem *p_item, int p_type, const String& p
p_item->set_range(1, obj->get( p_name ) );
} else {
/* FIXME: Why are both statements equal? */
p_item->set_range(1, obj->get( p_name ) );
}