Skip to content

/Cataclysm-DDA

Permalink
Browse files

Merge pull request #27569 from ZhilkinSerg/sa-2019-01-13 

Code optimizations reported by static code analysis (2019-01-13)
  • Loading branch information...
@kevingranade
kevingranade committed Jan 14, 2019
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
2 parents 9802051 + 78ea0ad commit 4d93e13a84ac8229c7cd03dadaed86ffe086e5b1
Unified Split
Showing with 403 additions and 416 deletions.
  1. +7 −7 src/activity_handlers.cpp
  2. +4 −5 src/activity_item_handling.cpp
  3. +24 −25 src/advanced_inv.cpp
  4. +3 −3 src/advanced_inv.h
  5. +2 −2 src/armor_layers.cpp
  6. +4 −4 src/artifact.cpp
  7. +4 −4 src/auto_pickup.cpp
  8. +1 −1 src/basecamp.h
  9. +2 −2 src/calendar.cpp
  10. +4 −4 src/cata_tiles.cpp
  11. +2 −2 src/cata_utility.cpp
  12. +2 −2 src/catacharset.cpp
  13. +3 −3 src/catalua.cpp
  14. +8 −8 src/character.cpp
  15. +3 −3 src/character.h
  16. +2 −2 src/clzones.cpp
  17. +15 −18 src/color.cpp
  18. +3 −4 src/computer.cpp
  19. +3 −3 src/creature.h
  20. +2 −2 src/dump.cpp
  21. +8 −9 src/editmap.cpp
  22. +2 −1 src/effect.cpp
  23. +22 −22 src/faction_camp.cpp
  24. +6 −6 src/field.cpp
  25. +1 −1 src/filesystem.cpp
  26. +1 −1 src/filesystem.h
  27. +8 −8 src/game.cpp
  28. +1 −1 src/game.h
  29. +4 −4 src/game_inventory.cpp
  30. +4 −4 src/gates.cpp
  31. +22 −24 src/iexamine.cpp
  32. +8 −9 src/inventory.cpp
  33. +3 −3 src/inventory_ui.cpp
  34. +7 −7 src/inventory_ui.h
  35. +26 −26 src/item.cpp
  36. +3 −3 src/item.h
  37. +9 −9 src/item_factory.cpp
  38. +2 −2 src/item_group.h
  39. +1 −1 src/item_search.cpp
  40. +1 −1 src/item_stack.cpp
  41. +1 −1 src/itype.cpp
  42. +9 −9 src/itype.h
  43. +4 −4 src/iuse.cpp
  44. +23 −25 src/iuse_actor.cpp
  45. +1 −1 src/iuse_actor.h
  46. +1 −1 src/iuse_software_minesweeper.cpp
  47. +7 −7 src/map.cpp
  48. +2 −2 src/map.h
  49. +6 −6 src/map_extras.cpp
  50. +2 −2 src/map_memory.h
  51. +3 −3 src/mapgen.cpp
  52. +7 −7 src/mapgen.h
  53. +1 −1 src/mapgen_functions.cpp
  54. +2 −2 src/mapgenformat.h
  55. +1 −1 src/mission.cpp
  56. +1 −1 src/mission_companion.cpp
  57. +3 −2 src/mission_companion.h
  58. +1 −1 src/mission_start.cpp
  59. +2 −2 src/mongroup.cpp
  60. +2 −2 src/mongroup.h
  61. +2 −2 src/monster.cpp
  62. +1 −1 src/monster.h
  63. +2 −2 src/morale.cpp
  64. +2 −2 src/npc.cpp
  65. +1 −1 src/npc.h
  66. +2 −2 src/npcmove.cpp
  67. +1 −1 src/npctalk.cpp
  68. +0 −4 src/npctalk.h
  69. +2 −2 src/npctrade.cpp
  70. +1 −1 src/overmap_ui.cpp
  71. +1 −1 src/path_info.h
  72. +3 −3 src/pickup.cpp
  73. +4 −2 src/player.cpp
  74. +11 −11 src/player_hardcoded_effects.cpp
  75. +1 −1 src/recipe_dictionary.cpp
  76. +1 −1 src/recipe_dictionary.h
  77. +7 −7 src/savegame_json.cpp
  78. +5 −5 src/sdltiles.cpp
  79. +7 −7 src/trait_group.h
  80. +1 −1 src/trapfunc.cpp
  81. +1 −0 src/tutorial.cpp
  82. +3 −4 src/uistate.h
  83. +4 −4 src/veh_interact.cpp
  84. +4 −4 src/veh_type.cpp
  85. +2 −2 src/veh_type.h
  86. +8 −8 src/vehicle.cpp
  87. +3 −3 src/vehicle.h
  88. +3 −4 src/vehicle_use.cpp
  89. +4 −4 src/weather.cpp
14 src/activity_handlers.cpp
View file
@@ -670,7 +670,7 @@ void butchery_drops_harvest( item *corpse_item, const mtype &mt, player &p, cons
roll = std::max<int>( corpse_damage_effect( roll, entry.type, corpse_item->damage_level( 4 ) ),
entry.base_num.first );
}
const itype *drop = NULL;
const itype *drop = nullptr;
if( entry.type != "bionic_group" ) {
drop = item::find_type( entry.drop );
}
@@ -1514,9 +1514,9 @@ void activity_handlers::longsalvage_finish( player_activity *act, player *p )
return;
}

for( auto it = items.begin(); it != items.end(); ++it ) {
if( actor->valid_to_cut_up( *it ) ) {
item_location item_loc( map_cursor( p->pos() ), &*it );
for( auto &item : items ) {
if( actor->valid_to_cut_up( item ) ) {
item_location item_loc( map_cursor( p->pos() ), &item );
actor->cut_up( *p, *salvage_tool, item_loc );
return;
}
@@ -1533,9 +1533,9 @@ void activity_handlers::make_zlave_finish( player_activity *act, player *p )
std::string corpse_name = act->str_values[0];
item *body = nullptr;

for( auto it = items.begin(); it != items.end(); ++it ) {
if( it->display_name() == corpse_name ) {
body = &*it;
for( auto &item : items ) {
if( item.display_name() == corpse_name ) {
body = &item;
}
}

9 src/activity_item_handling.cpp
View file
@@ -176,8 +176,7 @@ void put_into_vehicle( Character &c, item_drop_reason reason, const std::list<it

void stash_on_pet( const std::list<item> &items, monster &pet )
{
units::volume remaining_volume = pet.inv.empty() ? units::volume( 0 ) :
pet.inv.front().get_storage();
units::volume remaining_volume = pet.inv.empty() ? 0_ml : pet.inv.front().get_storage();
units::mass remaining_weight = pet.weight_capacity();

for( const auto &it : pet.inv ) {
@@ -399,7 +398,7 @@ std::list<act_item> reorder_for_dropping( const player &p, const drop_indexes &d
&& !second.it->is_worn_only_with( *first.it ) );
} );

units::volume storage_loss = 0; // Cumulatively increases
units::volume storage_loss = 0_ml; // Cumulatively increases
units::volume remaining_storage = p.volume_capacity(); // Cumulatively decreases

while( !worn.empty() && !inv.empty() ) {
@@ -469,7 +468,7 @@ std::list<item> obtain_activity_items( player_activity &act, player &p )
}
// Avoid tumbling to the ground. Unload cleanly.
const units::volume excessive_volume = p.volume_carried() - p.volume_capacity();
if( excessive_volume > 0 ) {
if( excessive_volume > 0_ml ) {
const auto excess = p.inv.remove_randomly_by_volume( excessive_volume );
res.insert( res.begin(), excess.begin(), excess.end() );
}
@@ -504,7 +503,7 @@ void activity_handlers::washing_finish( player_activity *act, player *p )

// Check again that we have enough water and soap incase the amount in our inventory changed somehow
// Consume the water and soap
units::volume total_volume = 0;
units::volume total_volume = 0_ml;

for( const act_item &filthy_item : items ) {
total_volume += filthy_item.it->volume();
49 src/advanced_inv.cpp
View file
@@ -231,28 +231,28 @@ void advanced_inventory::print_items( advanced_inventory_pane &pane, bool active
std::string volume_carried = format_volume( g->u.volume_carried() );
std::string volume_capacity = format_volume( g->u.volume_capacity() );
// align right, so calculate formatted head length
const std::string head = string_format( "%.1f/%.1f %s %s/%s %s",
const std::string formatted_head = string_format( "%.1f/%.1f %s %s/%s %s",
weight_carried, weight_capacity, weight_units(),
volume_carried.c_str(),
volume_capacity.c_str(),
volume_units_abbr() );
const int hrightcol = columns - 1 - head.length();
const int hrightcol = columns - 1 - formatted_head.length();
nc_color color = weight_carried > weight_capacity ? c_red : c_light_green;
mvwprintz( window, 4, hrightcol, color, "%.1f", weight_carried );
wprintz( window, c_light_gray, "/%.1f %s ", weight_capacity, weight_units() );
color = g->u.volume_carried().value() > g->u.volume_capacity().value() ? c_red : c_light_green;
wprintz( window, color, volume_carried.c_str() );
wprintz( window, c_light_gray, "/%s %s", volume_capacity.c_str(), volume_units_abbr() );
} else { //print square's current and total weight + volume
std::string head;
std::string formatted_head;
if( pane.get_area() == AIM_ALL ) {
head = string_format( "%3.1f %s %s %s",
formatted_head = string_format( "%3.1f %s %s %s",
convert_weight( squares[pane.get_area()].weight ),
weight_units(),
format_volume( squares[pane.get_area()].volume ).c_str(),
volume_units_abbr() );
} else {
units::volume maxvolume = 0;
units::volume maxvolume = 0_ml;
auto &s = squares[pane.get_area()];
if( pane.get_area() == AIM_CONTAINER && s.get_container( pane.in_vehicle() ) != nullptr ) {
maxvolume = s.get_container( pane.in_vehicle() )->get_container_capacity();
@@ -261,14 +261,14 @@ void advanced_inventory::print_items( advanced_inventory_pane &pane, bool active
} else {
maxvolume = g->m.max_volume( s.pos );
}
head = string_format( "%3.1f %s %s/%s %s",
formatted_head = string_format( "%3.1f %s %s/%s %s",
convert_weight( s.weight ),
weight_units(),
format_volume( s.volume ).c_str(),
format_volume( maxvolume ).c_str(),
volume_units_abbr() );
}
mvwprintz( window, 4, columns - 1 - head.length(), norm, head );
mvwprintz( window, 4, columns - 1 - formatted_head.length(), norm, formatted_head );
}

//print header row and determine max item name length
@@ -594,8 +594,8 @@ void advanced_inv_area::init()
pos = g->u.pos() + off;
veh = nullptr;
vstor = -1;
volume = 0; // must update in main function
weight = 0; // must update in main function
volume = 0_ml; // must update in main function
weight = 0_gram; // must update in main function
switch( id ) {
case AIM_INVENTORY:
case AIM_WORN:
@@ -887,8 +887,8 @@ void advanced_inventory_pane::add_items_from_area( advanced_inv_area &square,
bool vehicle_override )
{
assert( square.id != AIM_ALL );
square.volume = 0;
square.weight = 0;
square.volume = 0_ml;
square.weight = 0_gram;
if( !square.canputitems() ) {
return;
}
@@ -984,8 +984,8 @@ void advanced_inventory::recalc_pane( side p )
auto &alls = squares[AIM_ALL];
auto &there = panes[-p + 1];
auto &other = squares[there.get_area()];
alls.volume = 0;
alls.weight = 0;
alls.volume = 0_ml;
alls.weight = 0_gram;
for( auto &s : squares ) {
// All the surrounding squares, nothing else
if( s.id < AIM_SOUTHWEST || s.id > AIM_NORTHEAST ) {
@@ -1954,17 +1954,16 @@ bool advanced_inventory::query_destination( aim_location &def )
for( int i = AIM_SOUTHWEST; i <= AIM_NORTHEAST; i++ ) {
ordered_locs.push_back( screen_relative_location( static_cast <aim_location>( i ) ) );
}
for( std::vector <aim_location>::iterator iter = ordered_locs.begin(); iter != ordered_locs.end();
++iter ) {
auto &s = squares[*iter];
for( auto &ordered_loc : ordered_locs ) {
auto &s = squares[ordered_loc];
const int size = s.get_item_count();
std::string prefix = string_format( "%2d/%d", size, MAX_ITEM_IN_SQUARE );
if( size >= MAX_ITEM_IN_SQUARE ) {
prefix += _( " (FULL)" );
}
menu.addentry( *iter,
menu.addentry( ordered_loc,
( s.canputitems() && s.id != panes[src].get_area() ),
get_location_key( *iter ),
get_location_key( ordered_loc ),
prefix + " " + s.name + " " + ( s.veh != nullptr ? s.veh->name : "" ) );
}
}
@@ -2085,31 +2084,31 @@ bool advanced_inventory::move_content( item &src_container, item &dest_container
return false;
}

item &src = src_container.contents.front();
item &src_contents = src_container.contents.front();

if( !src.made_of( LIQUID ) ) {
if( !src_contents.made_of( LIQUID ) ) {
popup( _( "You can unload only liquids into target container." ) );
return false;
}

std::string err;
// @todo: Allow buckets here, but require them to be on the ground or wielded
const long amount = dest_container.get_remaining_capacity_for_liquid( src, false, &err );
const long amount = dest_container.get_remaining_capacity_for_liquid( src_contents, false, &err );
if( !err.empty() ) {
popup( err.c_str() );
return false;
}
if( src_container.is_non_resealable_container() ) {
if( src.charges > amount ) {
if( src_contents.charges > amount ) {
popup( _( "You can't partially unload liquids from unsealable container." ) );
return false;
}
src_container.on_contents_changed();
}
dest_container.fill_with( src, amount );
dest_container.fill_with( src_contents, amount );

uistate.adv_inv_container_content_type = dest_container.contents.front().typeId();
if( src.charges <= 0 ) {
if( src_contents.charges <= 0 ) {
src_container.contents.clear();
}

@@ -2183,7 +2182,7 @@ bool advanced_inventory::query_charges( aim_location destarea, const advanced_in
const units::mass unitweight = it.weight() / ( by_charges ? it.charges : 1 );
const units::mass max_weight = g->u.has_trait( trait_id( "DEBUG_STORAGE" ) ) ?
units::mass_max : g->u.weight_capacity() * 4 - g->u.weight_carried();
if( unitweight > 0 && ( unitweight * amount > max_weight ) ) {
if( unitweight > 0_gram && ( unitweight * amount > max_weight ) ) {
const long weightmax = max_weight / unitweight;
if( weightmax <= 0 ) {
popup( _( "This is too heavy!" ) );
6 src/advanced_inv.h
View file
@@ -101,7 +101,7 @@ struct advanced_inv_area {
advanced_inv_area( aim_location id, int hscreenx, int hscreeny, tripoint off, std::string name,
std::string shortname ) : id( id ), hscreenx( hscreenx ),
hscreeny( hscreeny ), off( off ), name( name ), shortname( shortname ), pos( 0, 0, 0 ),
canputitemsloc( false ), veh( nullptr ), vstor( -1 ), volume( 0 ), weight( 0 ),
canputitemsloc( false ), veh( nullptr ), vstor( -1 ), volume( 0_ml ), weight( 0_gram ),
max_size( 0 ) {
}

@@ -212,12 +212,12 @@ struct advanced_inv_listitem {
aim_location area, bool from_vehicle );
/**
* Create a normal item entry.
* @param items The list of item pointers.
* @param list The list of item pointers.
* @param index The index
* @param area The source area. Must not be AIM_ALL.
* @param from_vehicle Is the item from a vehicle cargo space?
*/
advanced_inv_listitem( const std::list<item *> &items, int index,
advanced_inv_listitem( const std::list<item *> &list, int index,
aim_location area, bool from_vehicle );
};

4 src/armor_layers.cpp
View file
@@ -540,13 +540,13 @@ void player::sort_armor()
mvwprintz( w_sort_left, drawindex + 1, 0, c_yellow, ">>" );
}

std::string name = tmp_worn[itemindex]->tname();
std::string worn_armor_name = tmp_worn[itemindex]->tname();
item_penalties const penalties =
get_item_penalties( tmp_worn[itemindex], *this, tabindex );

const int offset_x = ( itemindex == selected ) ? 3 : 2;
trim_and_print( w_sort_left, drawindex + 1, offset_x, left_w - offset_x - 3,
penalties.color_for_stacking_badness(), name );
penalties.color_for_stacking_badness(), worn_armor_name );
right_print( w_sort_left, drawindex + 1, 0, c_light_gray,
format_volume( tmp_worn[itemindex]->get_storage() ) );
}
8 src/artifact.cpp
View file
@@ -827,13 +827,13 @@ std::string new_artifact()
const artifact_armor_mod mod = random_entry_ref( info.available_mods );
if( mod != ARMORMOD_NULL ) {
const artifact_armor_form_datum &modinfo = artifact_armor_mod_data[mod];
if( modinfo.volume >= 0 || def.volume > -modinfo.volume ) {
if( modinfo.volume >= 0_ml || def.volume > -modinfo.volume ) {
def.volume += modinfo.volume;
} else {
def.volume = 250_ml;
}

if( modinfo.weight >= 0 || def.weight.value() > std::abs( modinfo.weight.value() ) ) {
if( modinfo.weight >= 0_gram || def.weight.value() > std::abs( modinfo.weight.value() ) ) {
def.weight += modinfo.weight;
} else {
def.weight = 1_gram;
@@ -860,10 +860,10 @@ std::string new_artifact()
}
def.armor->warmth += modinfo.warmth;

if( modinfo.storage > 0 || def.armor->storage > -modinfo.storage ) {
if( modinfo.storage > 0_ml || def.armor->storage > -modinfo.storage ) {
def.armor->storage += modinfo.storage;
} else {
def.armor->storage = 0;
def.armor->storage = 0_ml;
}

description << string_format( info.plural ?
8 src/auto_pickup.cpp
View file
@@ -617,13 +617,13 @@ void auto_pickup::refresh_map_items() const
} else {
//only re-exclude items from the existing mapping for now
//new exclusions will process during pickup attempts
for( auto iter = map_items.begin(); iter != map_items.end(); ++iter ) {
if( !check_special_rule( temp_items[ iter->first ]->materials, elem.sRule ) &&
!wildcard_match( iter->first, elem.sRule ) ) {
for( auto &map_item : map_items ) {
if( !check_special_rule( temp_items[ map_item.first ]->materials, elem.sRule ) &&
!wildcard_match( map_item.first, elem.sRule ) ) {
continue;
}

map_items[ iter->first ] = RULE_BLACKLISTED;
map_items[ map_item.first ] = RULE_BLACKLISTED;
}
}
}
2 src/basecamp.h
View file
@@ -141,7 +141,7 @@ class basecamp
* @param p NPC companion
* @param task
* @param omt_trg the overmap pos3 of the farm_ops
* @param ops whether to plow, plant, or harvest
* @param op whether to plow, plant, or harvest
*/
bool farm_return( npc &p, const std::string &task, const tripoint &omt_trg, farm_ops op );
void fortifications_return( npc &p );
4 src/calendar.cpp
View file
@@ -405,7 +405,7 @@ std::string to_string( const time_duration &d )
divider = 24_hours;
}

if( d % divider != 0 ) {
if( d % divider != 0_turns ) {
//~ %1$s - greater units of time (e.g. 3 hours), %2$s - lesser units of time (e.g. 11 minutes).
return string_format( _( "%1$s and %2$s" ),
to_string_clipped( d ),
@@ -433,7 +433,7 @@ std::string to_string_approx( const time_duration &dur, const bool verbose )
vicinity = 5_minutes;
} // Minutes and seconds can be estimated precisely.

if( divider != 0 ) {
if( divider != 0_turns ) {
const time_duration remainder = d % divider;

if( remainder >= divider - vicinity ) {
8 src/cata_tiles.cpp
View file
@@ -432,7 +432,7 @@ void tileset_loader::load_tileset( std::string img_path )
if( R >= 0 && R <= 255 && G >= 0 && G <= 255 && B >= 0 && B <= 255 ) {
const Uint32 key = SDL_MapRGB( tile_atlas->format, 0, 0, 0 );
throwErrorIf( SDL_SetColorKey( tile_atlas.get(), SDL_TRUE, key ) != 0, "SDL_SetColorKey failed" );
throwErrorIf( SDL_SetSurfaceRLE( tile_atlas.get(), true ), "SDL_SetSurfaceRLE failed" );
throwErrorIf( SDL_SetSurfaceRLE( tile_atlas.get(), 1 ), "SDL_SetSurfaceRLE failed" );
}

SDL_RendererInfo info;
@@ -1419,9 +1419,9 @@ void cata_tiles::init_minimap( int destx, int desty, int width, int height )
previous_submap_view = tripoint_min;

//allocate the textures for the texture pool
for( int i = 0; i < static_cast<int>( tex_pool.texture_pool.size() ); i++ ) {
tex_pool.texture_pool[i] = create_minimap_cache_texture( minimap_tile_size.x * SEEX,
minimap_tile_size.y * SEEY );
for( auto &i : tex_pool.texture_pool ) {
i = create_minimap_cache_texture( minimap_tile_size.x * SEEX,
minimap_tile_size.y * SEEY );
}
}

4 src/cata_utility.cpp
View file
@@ -228,7 +228,7 @@ double convert_weight( const units::mass &weight )

double convert_volume( int volume )
{
return convert_volume( volume, NULL );
return convert_volume( volume, nullptr );
}

double convert_volume( int volume, int *out_scale )
@@ -264,7 +264,7 @@ double temp_to_kelvin( double fahrenheit )

double clamp_to_width( double value, int width, int &scale )
{
return clamp_to_width( value, width, scale, NULL );
return clamp_to_width( value, width, scale, nullptr );
}

double clamp_to_width( double value, int width, int &scale, bool *out_truncated )
4 src/catacharset.cpp
View file
@@ -222,7 +222,7 @@ std::string utf8_truncate( std::string s, size_t length )
return s;
}

int last_pos = cursorx_to_position( s.c_str(), length, NULL, -1 );
int last_pos = cursorx_to_position( s.c_str(), length, nullptr, -1 );

return s.substr( 0, last_pos );
}
@@ -359,7 +359,7 @@ static void strip_trailing_nulls( std::string &str )
std::wstring utf8_to_wstr( const std::string &str )
{
#if defined(_WIN32) || defined(WINDOWS)
int sz = MultiByteToWideChar( CP_UTF8, 0, str.c_str(), -1, NULL, 0 ) + 1;
int sz = MultiByteToWideChar( CP_UTF8, 0, str.c_str(), -1, nullptr, 0 ) + 1;
std::wstring wstr( sz, '\0' );
MultiByteToWideChar( CP_UTF8, 0, str.c_str(), -1, &wstr[0], sz );
strip_trailing_nulls( wstr );

1 comment on commit 4d93e13

@kevingranade

This comment has been minimized.

Sign in to view
Copy link
Member Author

commented on 4d93e13 Jan 18, 2019

This commit has been mentioned on Cataclysm: Dark Days Ahead. There might be relevant details there:

https://discourse.cataclysmdda.org/t/which-experimental-version-wont-crash/18430/1
Please sign in to comment.
You can’t perform that action at this time.