Fix some minor code issues all over the place

19 files changed, 45 insertions, 109 deletions

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 78d551168..1d53fcffd 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -1,15 +1,9 @@
-cmake_minimum_required(VERSION 2.6)
-
-if(${CMAKE_VERSION} STREQUAL "2.8.2")
-	# Bug http://vtk.org/Bug/view.php?id=11020
-	message(WARNING "CMake/CPack version 2.8.2 will not create working .deb packages!")
-endif()
+cmake_minimum_required(VERSION 3.5)
 
 # This can be read from ${PROJECT_NAME} after project() is called
 project(minetest)
 set(PROJECT_NAME_CAPITALIZED "Minetest")
 
-# Works only for cmake 3.1 and greater
 set(CMAKE_CXX_STANDARD 11)
 set(GCC_MINIMUM_VERSION "4.8")
 set(CLANG_MINIMUM_VERSION "3.4")
diff --git a/lib/lua/CMakeLists.txt b/lib/lua/CMakeLists.txt
index 119dd6302..5d0dc0f70 100644
--- a/lib/lua/CMakeLists.txt
+++ b/lib/lua/CMakeLists.txt
@@ -1,5 +1,3 @@
-cmake_minimum_required(VERSION 2.4 FATAL_ERROR)
-
 project(lua C)
 
 set(LUA_VERSION_MAJOR 5)
@@ -15,9 +13,8 @@ set(LIBS)
 
 if(APPLE)
 	set(DEFAULT_POSIX TRUE)
-	set(DEFAULT_DLOPEN ON)
-	# use this on Mac OS X 10.3-
-	option(LUA_USE_MACOSX "Mac OS X 10.3-" OFF)
+	set(DEFAULT_DLOPEN OFF)
+	set(COMMON_CFLAGS "${COMMON_CFLAGS} -DLUA_USE_MACOSX")
 elseif(UNIX OR CYGWIN)
 	set(DEFAULT_POSIX TRUE)
 elseif(WIN32)
@@ -32,12 +29,6 @@ if(CMAKE_SYSTEM_NAME STREQUAL "Linux")
 	set(DEFAULT_DLOPEN ON)
 endif()
 
-# For "Mac OS X 10.3-"
-if(LUA_USE_MACOSX)
-	set(COMMON_CFLAGS "${COMMON_CFLAGS} -DLUA_USE_MACOSX")
-	set(LUA_USE_DLOPEN FALSE)
-endif(LUA_USE_MACOSX)
-
 option(LUA_USE_DLOPEN "Enable dlopen support." ${DEFAULT_DLOPEN})
 mark_as_advanced(LUA_USE_DLOPEN)
 
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index b8ce69f1d..b6bba6e8d 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -1,5 +1,3 @@
-cmake_minimum_required(VERSION 2.6)
-
 project(minetest)
 
 INCLUDE(CheckIncludeFiles)
@@ -124,27 +122,6 @@ option(ENABLE_FREETYPE "Enable FreeType2 (TrueType fonts and basic unicode suppo
 set(USE_FREETYPE FALSE)
 
 if(ENABLE_FREETYPE)
-##
-## Note: FindFreetype.cmake seems to have been fixed in recent versions of
-## CMake. If issues persist, re-enable this workaround specificially for the
-## failing platforms.
-##
-#	if(UNIX)
-#		include(FindPkgConfig)
-#		if(PKG_CONFIG_FOUND)
-#			pkg_check_modules(FREETYPE QUIET freetype2)
-#			if(FREETYPE_FOUND)
-#				SET(FREETYPE_PKGCONFIG_FOUND TRUE)
-#				SET(FREETYPE_LIBRARY ${FREETYPE_LIBRARIES})
-#				# Because CMake is idiotic
-#				string(REPLACE ";" " " FREETYPE_CFLAGS_STR ${FREETYPE_CFLAGS})
-#				string(REPLACE ";" " " FREETYPE_LDFLAGS_STR ${FREETYPE_LDFLAGS})
-#			endif(FREETYPE_FOUND)
-#		endif(PKG_CONFIG_FOUND)
-#	endif(UNIX)
-#	if(NOT FREETYPE_FOUND)
-#		find_package(Freetype)
-#	endif()
 	find_package(Freetype)
 	if(FREETYPE_FOUND)
 		message(STATUS "Freetype enabled.")
diff --git a/src/client/content_cao.cpp b/src/client/content_cao.cpp
index c645900aa..c65977b44 100644
--- a/src/client/content_cao.cpp
+++ b/src/client/content_cao.cpp
@@ -1165,7 +1165,7 @@ void GenericCAO::step(float dtime, ClientEnvironment *env)
 		}
 	}
 
-	if (!getParent() && std::fabs(m_prop.automatic_rotate) > 0.001) {
+	if (!getParent() && node && fabs(m_prop.automatic_rotate) > 0.001f) {
 		// This is the child node's rotation. It is only used for automatic_rotate.
 		v3f local_rot = node->getRotation();
 		local_rot.Y = modulo360f(local_rot.Y - dtime * core::RADTODEG *
@@ -1174,7 +1174,7 @@ void GenericCAO::step(float dtime, ClientEnvironment *env)
 	}
 
 	if (!getParent() && m_prop.automatic_face_movement_dir &&
-			(fabs(m_velocity.Z) > 0.001 || fabs(m_velocity.X) > 0.001)) {
+			(fabs(m_velocity.Z) > 0.001f || fabs(m_velocity.X) > 0.001f)) {
 		float target_yaw = atan2(m_velocity.Z, m_velocity.X) * 180 / M_PI
 				+ m_prop.automatic_face_movement_dir_offset;
 		float max_rotation_per_sec =
diff --git a/src/client/game.cpp b/src/client/game.cpp
index 6151d2aa6..9e942f47a 100644
--- a/src/client/game.cpp
+++ b/src/client/game.cpp
@@ -186,7 +186,7 @@ struct LocalFormspecHandler : public TextDest
 			return;
 		}
 
-		if (m_client && m_client->modsLoaded())
+		if (m_client->modsLoaded())
 			m_client->getScript()->on_formspec_input(m_formname, fields);
 	}
 
@@ -583,7 +583,7 @@ public:
 
 	virtual IShaderConstantSetter* create()
 	{
-		GameGlobalShaderConstantSetter *scs = new GameGlobalShaderConstantSetter(
+		auto *scs = new GameGlobalShaderConstantSetter(
 				m_sky, m_force_fog_off, m_fog_range, m_client);
 		if (!m_sky)
 			created_nosky.push_back(scs);
@@ -1338,7 +1338,7 @@ bool Game::createClient(const GameStartData &start_data)
 		return false;
 	}
 
-	GameGlobalShaderConstantSetterFactory *scsf = new GameGlobalShaderConstantSetterFactory(
+	auto *scsf = new GameGlobalShaderConstantSetterFactory(
 			&m_flags.force_fog_off, &runData.fog_range, client);
 	shader_src->addShaderConstantSetterFactory(scsf);
 
@@ -1348,20 +1348,14 @@ bool Game::createClient(const GameStartData &start_data)
 	/* Camera
 	 */
 	camera = new Camera(*draw_control, client);
-	if (!camera || !camera->successfullyCreated(*error_message))
+	if (!camera->successfullyCreated(*error_message))
 		return false;
 	client->setCamera(camera);
 
 	/* Clouds
 	 */
-	if (m_cache_enable_clouds) {
+	if (m_cache_enable_clouds)
 		clouds = new Clouds(smgr, -1, time(0));
-		if (!clouds) {
-			*error_message = "Memory allocation error (clouds)";
-			errorstream << *error_message << std::endl;
-			return false;
-		}
-	}
 
 	/* Skybox
 	 */
@@ -1369,12 +1363,6 @@ bool Game::createClient(const GameStartData &start_data)
 	scsf->setSky(sky);
 	skybox = NULL;	// This is used/set later on in the main run loop
 
-	if (!sky) {
-		*error_message = "Memory allocation error sky";
-		errorstream << *error_message << std::endl;
-		return false;
-	}
-
 	/* Pre-calculated values
 	 */
 	video::ITexture *t = texture_src->getTexture("crack_anylength.png");
@@ -1404,12 +1392,6 @@ bool Game::createClient(const GameStartData &start_data)
 
 	hud = new Hud(guienv, client, player, &player->inventory);
 
-	if (!hud) {
-		*error_message = "Memory error: could not create HUD";
-		errorstream << *error_message << std::endl;
-		return false;
-	}
-
 	mapper = client->getMinimap();
 
 	if (mapper && client->modsLoaded())
@@ -1431,11 +1413,6 @@ bool Game::initGui()
 	// Chat backend and console
 	gui_chat_console = new GUIChatConsole(guienv, guienv->getRootGUIElement(),
 			-1, chat_backend, client, &g_menumgr);
-	if (!gui_chat_console) {
-		*error_message = "Could not allocate memory for chat console";
-		errorstream << *error_message << std::endl;
-		return false;
-	}
 
 #ifdef HAVE_TOUCHSCREENGUI
 
@@ -1492,9 +1469,6 @@ bool Game::connectToServer(const GameStartData &start_data,
 			itemdef_manager, nodedef_manager, sound, eventmgr,
 			connect_address.isIPv6(), m_game_ui.get());
 
-	if (!client)
-		return false;
-
 	client->m_simple_singleplayer_mode = simple_singleplayer_mode;
 
 	infostream << "Connecting to server at ";
diff --git a/src/client/hud.cpp b/src/client/hud.cpp
index 8d8411ca1..e956c2738 100644
--- a/src/client/hud.cpp
+++ b/src/client/hud.cpp
@@ -1055,9 +1055,9 @@ void drawItemStack(
 
 	if (def.type == ITEM_TOOL && item.wear != 0) {
 		// Draw a progressbar
-		float barheight = rect.getHeight() / 16;
-		float barpad_x = rect.getWidth() / 16;
-		float barpad_y = rect.getHeight() / 16;
+		float barheight = static_cast<float>(rect.getHeight()) / 16;
+		float barpad_x = static_cast<float>(rect.getWidth()) / 16;
+		float barpad_y = static_cast<float>(rect.getHeight()) / 16;
 
 		core::rect<s32> progressrect(
 			rect.UpperLeftCorner.X + barpad_x,
diff --git a/src/client/mapblock_mesh.cpp b/src/client/mapblock_mesh.cpp
index dac25a066..4c43fcb61 100644
--- a/src/client/mapblock_mesh.cpp
+++ b/src/client/mapblock_mesh.cpp
@@ -1201,13 +1201,13 @@ MapBlockMesh::MapBlockMesh(MeshMakeData *data, v3s16 camera_offset):
 MapBlockMesh::~MapBlockMesh()
 {
 	for (scene::IMesh *m : m_mesh) {
-		if (m_enable_vbo && m)
+		if (m_enable_vbo) {
 			for (u32 i = 0; i < m->getMeshBufferCount(); i++) {
 				scene::IMeshBuffer *buf = m->getMeshBuffer(i);
 				RenderingEngine::get_video_driver()->removeHardwareBuffer(buf);
 			}
+		}
 		m->drop();
-		m = NULL;
 	}
 	delete m_minimap_mapblock;
 }
diff --git a/src/client/tile.cpp b/src/client/tile.cpp
index d03588b2b..37836d0df 100644
--- a/src/client/tile.cpp
+++ b/src/client/tile.cpp
@@ -1633,6 +1633,13 @@ bool TextureSource::generateImagePart(std::string part_of_name,
 			/* IMPORTANT: When changing this, getTextureForMesh() needs to be
 			 * updated too. */
 
+			if (!baseimg) {
+				errorstream << "generateImagePart(): baseimg == NULL "
+						<< "for part_of_name=\"" << part_of_name
+						<< "\", cancelling." << std::endl;
+				return false;
+			}
+
 			// Apply the "clean transparent" filter, if configured.
 			if (g_settings->getBool("texture_clean_transparent"))
 				imageCleanTransparent(baseimg, 127);
diff --git a/src/filesys.cpp b/src/filesys.cpp
index 2470b1b64..28a33f4d0 100644
--- a/src/filesys.cpp
+++ b/src/filesys.cpp
@@ -295,31 +295,26 @@ bool RecursiveDelete(const std::string &path)
 
 	infostream<<"Removing \""<<path<<"\""<<std::endl;
 
-	//return false;
-
 	pid_t child_pid = fork();
 
 	if(child_pid == 0)
 	{
 		// Child
-		char argv_data[3][10000];
+		const char *argv[4] = {
 #ifdef __ANDROID__
-		strcpy(argv_data[0], "/system/bin/rm");
+			"/system/bin/rm",
 #else
-		strcpy(argv_data[0], "/bin/rm");
+			"/bin/rm",
 #endif
-		strcpy(argv_data[1], "-rf");
-		strncpy(argv_data[2], path.c_str(), sizeof(argv_data[2]) - 1);
-		char *argv[4];
-		argv[0] = argv_data[0];
-		argv[1] = argv_data[1];
-		argv[2] = argv_data[2];
-		argv[3] = NULL;
+			"-rf",
+			path.c_str(),
+			NULL
+		};
 
 		verbosestream<<"Executing '"<<argv[0]<<"' '"<<argv[1]<<"' '"
 				<<argv[2]<<"'"<<std::endl;
 
-		execv(argv[0], argv);
+		execv(argv[0], const_cast<char**>(argv));
 
 		// Execv shouldn't return. Failed.
 		_exit(1);
@@ -331,7 +326,6 @@ bool RecursiveDelete(const std::string &path)
 		pid_t tpid;
 		do{
 			tpid = wait(&child_status);
-			//if(tpid != child_pid) process_terminated(tpid);
 		}while(tpid != child_pid);
 		return (child_status == 0);
 	}
diff --git a/src/gui/touchscreengui.cpp b/src/gui/touchscreengui.cpp
index 0d64aa618..e1a971462 100644
--- a/src/gui/touchscreengui.cpp
+++ b/src/gui/touchscreengui.cpp
@@ -881,8 +881,7 @@ void TouchScreenGUI::translateEvent(const SEvent &event)
 			s32 dyj = event.TouchInput.Y - m_screensize.Y + button_size * 5.0f / 2.0f;
 			bool inside_joystick = (dxj * dxj + dyj * dyj <= button_size * button_size * 1.5 * 1.5);
 
-			if (m_joystick_has_really_moved ||
-					(!m_joystick_has_really_moved && inside_joystick) ||
+			if (m_joystick_has_really_moved || inside_joystick ||
 					(!m_fixed_joystick &&
 					distance_sq > m_touchscreen_threshold * m_touchscreen_threshold)) {
 				m_joystick_has_really_moved = true;
diff --git a/src/irrlicht_changes/CGUITTFont.h b/src/irrlicht_changes/CGUITTFont.h
index cf64934a2..310f74f67 100644
--- a/src/irrlicht_changes/CGUITTFont.h
+++ b/src/irrlicht_changes/CGUITTFont.h
@@ -356,7 +356,7 @@ namespace gui
 				load_flags = FT_LOAD_DEFAULT | FT_LOAD_RENDER;
 				if (!useHinting()) load_flags |= FT_LOAD_NO_HINTING;
 				if (!useAutoHinting()) load_flags |= FT_LOAD_NO_AUTOHINT;
-				if (useMonochrome()) load_flags |= FT_LOAD_MONOCHROME | FT_LOAD_TARGET_MONO | FT_RENDER_MODE_MONO;
+				if (useMonochrome()) load_flags |= FT_LOAD_MONOCHROME | FT_LOAD_TARGET_MONO;
 				else load_flags |= FT_LOAD_TARGET_NORMAL;
 			}
 			u32 getWidthFromCharacter(wchar_t c) const;
diff --git a/src/irrlicht_changes/irrUString.h b/src/irrlicht_changes/irrUString.h
index b628c092c..09172ee6d 100644
--- a/src/irrlicht_changes/irrUString.h
+++ b/src/irrlicht_changes/irrUString.h
@@ -1331,7 +1331,7 @@ public:
 	{
 		u32 i;
 		const uchar16_t* oa = other.c_str();
-		for(i=0; array[i] && oa[i] && i < n; ++i)
+		for(i=0; i < n && array[i] && oa[i]; ++i)
 			if (array[i] != oa[i])
 				return false;
 
@@ -1350,7 +1350,7 @@ public:
 		if (!str)
 			return false;
 		u32 i;
-		for(i=0; array[i] && str[i] && i < n; ++i)
+		for(i=0; i < n && array[i] && str[i]; ++i)
 			if (array[i] != str[i])
 				return false;
 
diff --git a/src/mapgen/mapgen_v7.cpp b/src/mapgen/mapgen_v7.cpp
index cc5f5726d..91f004518 100644
--- a/src/mapgen/mapgen_v7.cpp
+++ b/src/mapgen/mapgen_v7.cpp
@@ -297,7 +297,7 @@ int MapgenV7::getSpawnLevelAtPoint(v2s16 p)
 	int iters = 256;
 	while (iters > 0 && y <= max_spawn_y) {
 		if (!getMountainTerrainAtPoint(p.X, y + 1, p.Y)) {
-			if (y <= water_level || y > max_spawn_y)
+			if (y <= water_level)
 				return MAX_MAP_GENERATION_LIMIT; // Unsuitable spawn point
 
 			// y + 1 due to biome 'dust'
diff --git a/src/mapgen/treegen.cpp b/src/mapgen/treegen.cpp
index e633d800a..ec7771439 100644
--- a/src/mapgen/treegen.cpp
+++ b/src/mapgen/treegen.cpp
@@ -406,7 +406,8 @@ treegen::error make_ltree(MMVManip &vmanip, v3s16 p0,
 					v3f(position.X, position.Y, position.Z - 1),
 					tree_definition
 				);
-			} if (!stack_orientation.empty()) {
+			}
+			if (!stack_orientation.empty()) {
 				s16 size = 1;
 				for (x = -size; x <= size; x++)
 				for (y = -size; y <= size; y++)
diff --git a/src/network/serverpackethandler.cpp b/src/network/serverpackethandler.cpp
index 3db4eb286..c636d01e1 100644
--- a/src/network/serverpackethandler.cpp
+++ b/src/network/serverpackethandler.cpp
@@ -316,7 +316,7 @@ void Server::handleCommand_Init2(NetworkPacket* pkt)
 	// Send active objects
 	{
 		PlayerSAO *sao = getPlayerSAO(peer_id);
-		if (client && sao)
+		if (sao)
 			SendActiveObjectRemoveAdd(client, sao);
 	}
 
diff --git a/src/nodedef.cpp b/src/nodedef.cpp
index 80013192d..f9d15a9f6 100644
--- a/src/nodedef.cpp
+++ b/src/nodedef.cpp
@@ -617,7 +617,7 @@ static void fillTileAttribs(ITextureSource *tsrc, TileLayer *layer,
 	bool has_scale = tiledef.scale > 0;
 	bool use_autoscale = tsettings.autoscale_mode == AUTOSCALE_FORCE ||
 		(tsettings.autoscale_mode == AUTOSCALE_ENABLE && !has_scale);
-	if (use_autoscale && layer->texture) {
+	if (use_autoscale) {
 		auto texture_size = layer->texture->getOriginalSize();
 		float base_size = tsettings.node_texture_size;
 		float size = std::fmin(texture_size.Width, texture_size.Height);
diff --git a/src/script/cpp_api/s_security.cpp b/src/script/cpp_api/s_security.cpp
index 01333b941..63058d7c3 100644
--- a/src/script/cpp_api/s_security.cpp
+++ b/src/script/cpp_api/s_security.cpp
@@ -398,10 +398,9 @@ bool ScriptApiSecurity::safeLoadFile(lua_State *L, const char *path, const char
 			lua_pushfstring(L, "%s: %s", path, strerror(errno));
 			return false;
 		}
-		chunk_name = new char[strlen(display_name) + 2];
-		chunk_name[0] = '@';
-		chunk_name[1] = '\0';
-		strcat(chunk_name, display_name);
+		size_t len = strlen(display_name) + 2;
+		chunk_name = new char[len];
+		snprintf(chunk_name, len, "@%s", display_name);
 	}
 
 	size_t start = 0;
diff --git a/src/util/srp.cpp b/src/util/srp.cpp
index f4d369d68..ceb2fef9e 100644
--- a/src/util/srp.cpp
+++ b/src/util/srp.cpp
@@ -1015,10 +1015,10 @@ void  srp_user_process_challenge(struct SRPUser *usr,
 			goto cleanup_and_exit;
 
 		*bytes_M = usr->M;
-		if (len_M) *len_M = hash_length(usr->hash_alg);
+		*len_M = hash_length(usr->hash_alg);
 	} else {
 		*bytes_M = NULL;
-		if (len_M) *len_M = 0;
+		*len_M = 0;
 	}
 
 cleanup_and_exit:
diff --git a/src/util/string.cpp b/src/util/string.cpp
index 8381a29c5..3ac3b8cf0 100644
--- a/src/util/string.cpp
+++ b/src/util/string.cpp
@@ -633,7 +633,7 @@ static bool parseNamedColorString(const std::string &value, video::SColor &color
 		color_name = value;
 	}
 
-	color_name = lowercase(value);
+	color_name = lowercase(color_name);
 
 	std::map<const std::string, unsigned>::const_iterator it;
 	it = named_colors.colors.find(color_name);