Fix errors found by professional static code analyzer

[akozlovskiy119]

Not so long ago Minetest was checked by the professional static code analyzer called PVS-Studio. The developers published some of the errors they have found in their blog: https://www.viva64.com/en/b/0751/#ID2E6FF31CDC. I think Minetest developers should take them into account and, if possible, fix them.

Please note that I am not a representative of PVS-Studio. I came across this article by chance and thought I should inform developers about the errors. The blog also recommends a full analysis but I'm not sure Minetest can afford it.

[SmallJoker]

color_name will be fixed by #10425
nearest_emergefull_d might be interesting to @lhofhansl
max_spawn_y garbage caused by double-checking. not a problem
m_client Edge case that might never happen. The client is always destructed after processing events.
FT_RENDER_MODE_NORMAL interesting. This affects the font rendering. Everything looks well there, thus not so important
getHeight() / 16 is the cause of #7852. sweet.
make_ltree no idea about this stuff. It seems to work well so far.
equalsn error imported from Irrlicht. should either be fixed or removed (AFAIK it's an unused function)

[akozlovskiy119]

I also checked Minetest with TscanCode static analyzer found on Github: https://github.com/Tencent/TscanCode. I think it is appropriate to post some results in this issue. You can see the full log at https://gist.github.com/akozlovskiy119/e7fe6160467a0d2abf2dcb0dff77927d.

Interestingly, this analyzer also supports Lua, it may be worth checking minetest_game with it later.

---

1. src/client/content_cao.cpp: Comparing node to null at line 1087 implies that node might be null. Dereferencing null pointer node.

void GenericCAO::step(float dtime, ClientEnvironment *env)
{
	...

	// Make sure m_is_visible is always applied
	scene::ISceneNode *node = getSceneNode();
=>	if (node)
		node->setVisible(m_is_visible);

	...

	if (!getParent() && std::fabs(m_prop.automatic_rotate) > 0.001) {
		// 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 *
				m_prop.automatic_rotate);
=>		node->setRotation(local_rot);
	}

	...
}

2. src/client/mapblock_mesh.cpp: Comparing m to null at line 1204 implies that m might be null. Dereferencing null pointer m.

MapBlockMesh::~MapBlockMesh()
{
	for (scene::IMesh *m : m_mesh) {
=>		if (m_enable_vbo && m)
			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;
}

3. src/client/tile.cpp: baseimg is not checked for null in this if-else branch at line 1631, but checked in all others.

bool TextureSource::generateImagePart(std::string part_of_name,
		video::IImage *& baseimg)
{
	...

		else if (str_starts_with(part_of_name, "[applyfiltersformesh"))
		{
			/* IMPORTANT: When changing this, getTextureForMesh() needs to be
			 * updated too. */

			// Apply the "clean transparent" filter, if configured.
			if (g_settings->getBool("texture_clean_transparent"))
=>				imageCleanTransparent(baseimg, 127);

			...
		}

	...
}

imageCleanTransparent also doesn't check if src is null.

void imageCleanTransparent(video::IImage *src, u32 threshold)
{
=>	core::dimension2d<u32> dim = src->getDimension();

	...
}

4. src/network/serverpackethandler.cpp: Null-checking client at line 319 suggests that it may be null, but it has already been dereferenced at line 314.

void Server::handleCommand_Init2(NetworkPacket* pkt)
{
	...

=>	RemoteClient *client = getClient(peer_id, CS_InitDone);

	// Keep client language for server translations
=>	client->setLangCode(lang);

	// Send active objects
	{
		PlayerSAO *sao = getPlayerSAO(peer_id);
=>		if (client && sao)
			SendActiveObjectRemoveAdd(client, sao);
	}

	...

	// Warnings about protocol version can be issued here
=>	if (client->net_proto_version < LATEST_PROTOCOL_VERSION) {
		SendChatMessage(peer_id, ChatMessage(CHATMESSAGE_TYPE_SYSTEM,
			L"# Server: WARNING: YOUR CLIENT'S VERSION MAY NOT BE FULLY COMPATIBLE "
			L"WITH THIS SERVER!"));
	}
}

5. src/nodedef.cpp: Comparing layer->texture to null at line 620 implies that layer->texture might be null. Dereferencing null pointer layer->texture.

static void fillTileAttribs(ITextureSource *tsrc, TileLayer *layer,
		const TileSpec &tile, const TileDef &tiledef, video::SColor color,
		u8 material_type, u32 shader_id, bool backface_culling,
		const TextureSettings &tsettings)
{
	...
	
=>	if (use_autoscale && layer->texture) {
		...

	if (layer->material_flags & MATERIAL_FLAG_ANIMATION) {
		...
=>		tiledef.animation.determineParams(layer->texture->getOriginalSize(),
				&frame_count, &frame_length_ms, NULL);
		...
	}

	...

		for (int i = 0; i < frame_count; i++) {
			...
			tiledef.animation.getTextureModifer(os,
=>					layer->texture->getOriginalSize(), i);
			...
		}

	...
}

6. src/gui/guiScrollBar.cpp: Comparing Parent to null at lines 128 and 175 suggests that it may be null. Dereferencing Parent without comparing to null at lines 71, 89 and 110.

7. src/gui/guiFormSpecMenu.cpp: Condition at lines 2707 and 2708 can be simplified to parts.size() <= 2 || m_formspec_version > FORMSPEC_API_VERSION.

8. src/gui/touchscreengui.cpp: Part of the condition at lines 884 and 885 can be simplified to m_joystick_has_really_moved || inside_joystick.

9. src/irrlicht_changes/irrUString.h: Array index i is used before limits check at lines 1334 and 1353. Same as in irrString.h, equalsn is never used.

[numberZero]

“FT_RENDER_MODE_NORMAL” is by itself a false positive, FT_LOAD_TARGET_NORMAL is not strictly a flag but a shifted enumerator (so that exactly one of FT_LOAD_TARGET_* can be packed into flags) so the fact it is zero is okay (as long as no other FT_LOAD_TARGET_* is set in the flags). On the other hand, ORing FT_RENDER_MODE_MONO is wrong, that’s not a flag at all and becomes FT_LOAD_NO_HINTING effectively.

minetest/src/irrlicht_changes/CGUITTFont.h

Lines 359 to 360 in 0750047

	if (useMonochrome()) load_flags |= FT_LOAD_MONOCHROME | FT_LOAD_TARGET_MONO | FT_RENDER_MODE_MONO;
	else load_flags |= FT_LOAD_TARGET_NORMAL;