Merge branch 'master' into doc-refactor-2

.github/workflows/linux.yml

@@ -86,7 +86,7 @@ jobs:
       - name: Install deps
         run: |
           source ./util/ci/common.sh
-          install_linux_deps clang-7 valgrind
+          install_linux_deps clang-7 llvm
 
       - name: Build
         run: |
@@ -94,15 +94,12 @@ jobs:
         env:
           CC: clang-7
           CXX: clang++-7
+          CMAKE_FLAGS: '-DCMAKE_C_FLAGS="-fsanitize=address" -DCMAKE_CXX_FLAGS="-fsanitize=address"'
 
       - name: Unittest
         run: |
           ./bin/minetest --run-unittests
 
-      - name: Valgrind
-        run: |
-          valgrind --leak-check=full --leak-check-heuristics=all --undef-value-errors=no --error-exitcode=9 ./bin/minetest --run-unittests
-
   # Current clang version
   clang_14:
     runs-on: ubuntu-22.04

doc/lua_api.md

@@ -5874,8 +5874,11 @@ handler.
 * `minetest.add_entity(pos, name, [staticdata])`: Spawn Lua-defined entity at
   position.
     * Returns `ObjectRef`, or `nil` if failed
+    * Entities with `static_save = true` can be added also 
+      to unloaded and non-generated blocks.
 * `minetest.add_item(pos, item)`: Spawn item
     * Returns `ObjectRef`, or `nil` if failed
+    * Items can be added also to unloaded and non-generated blocks.
 * `minetest.get_player_by_name(name)`: Get an `ObjectRef` to a player
 * `minetest.get_objects_inside_radius(pos, radius)`: returns a list of
   ObjectRefs.

src/client/clientmedia.cpp

@@ -29,6 +29,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "util/serialize.h"
 #include "util/sha1.h"
 #include "util/string.h"
+#include <sstream>
 
 static std::string getMediaCacheDir()
 {
@@ -41,7 +42,16 @@ bool clientMediaUpdateCache(const std::string &raw_hash, const std::string &file
 	std::string sha1_hex = hex_encode(raw_hash);
 	if (!media_cache.exists(sha1_hex))
 		return media_cache.update(sha1_hex, filedata);
-	return true;
+	return false;
+}
+
+bool clientMediaUpdateCacheCopy(const std::string &raw_hash, const std::string &path)
+{
+	FileCache media_cache(getMediaCacheDir());
+	std::string sha1_hex = hex_encode(raw_hash);
+	if (!media_cache.exists(sha1_hex))
+		return media_cache.updateCopyFile(sha1_hex, path);
+	return false;
 }
 
 /*
@@ -189,10 +199,6 @@ void ClientMediaDownloader::initialStep(Client *client)
 
 	assert(m_uncached_received_count == 0);
 
-	// Create the media cache dir if we are likely to write to it
-	if (m_uncached_count != 0)
-		createCacheDirs();
-
 	// If we found all files in the cache, report this fact to the server.
 	// If the server reported no remote servers, immediately start
 	// conventional transfers. Note: if cURL support is not compiled in,
@@ -511,18 +517,6 @@ IClientMediaDownloader::IClientMediaDownloader():
 {
 }
 
-void IClientMediaDownloader::createCacheDirs()
-{
-	if (!m_write_to_cache)
-		return;
-
-	std::string path = getMediaCacheDir();
-	if (!fs::CreateAllDirs(path)) {
-		errorstream << "Client: Could not create media cache directory: "
-			<< path << std::endl;
-	}
-}
-
 bool IClientMediaDownloader::tryLoadFromCache(const std::string &name,
 	const std::string &sha1, Client *client)
 {
@@ -726,8 +720,6 @@ void SingleMediaDownloader::initialStep(Client *client)
 	if (isDone())
 		return;
 
-	createCacheDirs();
-
 	// If the server reported no remote servers, immediately fall back to
 	// conventional transfer.
 	if (!USE_CURL || m_remotes.empty()) {

src/client/clientmedia.h

@@ -22,7 +22,6 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "irrlichttypes.h"
 #include "filecache.h"
 #include "util/basic_macros.h"
-#include <ostream>
 #include <map>
 #include <set>
 #include <vector>
@@ -35,10 +34,15 @@ struct HTTPFetchResult;
 #define MTHASHSET_FILE_NAME "index.mth"
 
 // Store file into media cache (unless it exists already)
-// Validating the hash is responsibility of the caller
+// Caller should check the hash.
+// return true if something was updated
 bool clientMediaUpdateCache(const std::string &raw_hash,
 	const std::string &filedata);
 
+// Copy file on disk(!) into media cache (unless it exists already)
+bool clientMediaUpdateCacheCopy(const std::string &raw_hash,
+	const std::string &path);
+
 // more of a base class than an interface but this name was most convenient...
 class IClientMediaDownloader
 {
@@ -81,8 +85,6 @@ protected:
 	virtual bool loadMedia(Client *client, const std::string &data,
 		const std::string &name) = 0;
 
-	void createCacheDirs();
-
 	bool tryLoadFromCache(const std::string &name, const std::string &sha1,
 			Client *client);
 

src/client/filecache.cpp

@@ -28,6 +28,14 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include <fstream>
 #include <cstdlib>
 
+void FileCache::createDir()
+{
+	if (!fs::CreateAllDirs(m_dir)) {
+		errorstream << "Could not create cache directory: "
+			<< m_dir << std::endl;
+	}
+}
+
 bool FileCache::loadByPath(const std::string &path, std::ostream &os)
 {
 	std::ifstream fis(path.c_str(), std::ios_base::binary);
@@ -40,8 +48,8 @@ bool FileCache::loadByPath(const std::string &path, std::ostream &os)
 
 	bool bad = false;
 	for(;;){
-		char buf[1024];
-		fis.read(buf, 1024);
+		char buf[4096];
+		fis.read(buf, sizeof(buf));
 		std::streamsize len = fis.gcount();
 		os.write(buf, len);
 		if(fis.eof())
@@ -61,6 +69,7 @@ bool FileCache::loadByPath(const std::string &path, std::ostream &os)
 
 bool FileCache::updateByPath(const std::string &path, const std::string &data)
 {
+	createDir();
 	std::ofstream file(path.c_str(), std::ios_base::binary |
 			std::ios_base::trunc);
 
@@ -95,3 +104,11 @@ bool FileCache::exists(const std::string &name)
 	std::ifstream fis(path.c_str(), std::ios_base::binary);
 	return fis.good();
 }
+
+bool FileCache::updateCopyFile(const std::string &name, const std::string &src_path)
+{
+	std::string path = m_dir + DIR_DELIM + name;
+
+	createDir();
+	return fs::CopyFileContents(src_path, path);
+}

src/client/filecache.h

@@ -35,9 +35,13 @@ public:
 	bool load(const std::string &name, std::ostream &os);
 	bool exists(const std::string &name);
 
+	// Copy another file on disk into the cache
+	bool updateCopyFile(const std::string &name, const std::string &src_path);
+
 private:
 	std::string m_dir;
 
+	void createDir();
 	bool loadByPath(const std::string &path, std::ostream &os);
 	bool updateByPath(const std::string &path, const std::string &data);
 };

src/client/game.cpp

@@ -33,6 +33,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "client/mapblock_mesh.h"
 #include "client/sound.h"
 #include "clientmap.h"
+#include "clientmedia.h" // For clientMediaUpdateCacheCopy
 #include "clouds.h"
 #include "config.h"
 #include "content_cao.h"
@@ -769,6 +770,7 @@ protected:
 	bool initSound();
 	bool createSingleplayerServer(const std::string &map_dir,
 			const SubgameSpec &gamespec, u16 port);
+	void copyServerClientCache();
 
 	// Client creation
 	bool createClient(const GameStartData &start_data);
@@ -1453,9 +1455,31 @@ bool Game::createSingleplayerServer(const std::string &map_dir,
 			false, nullptr, error_message);
 	server->start();
 
+	copyServerClientCache();
+
 	return true;
 }
 
+void Game::copyServerClientCache()
+{
+	// It would be possible to let the client directly read the media files
+	// from where the server knows they are. But aside from being more complicated
+	// it would also *not* fill the media cache and cause slower joining of 
+	// remote servers.
+	// (Imagine that you launch a game once locally and then connect to a server.)
+
+	assert(server);
+	auto map = server->getMediaList();
+	u32 n = 0;
+	for (auto &it : map) {
+		assert(it.first.size() == 20); // SHA1
+		if (clientMediaUpdateCacheCopy(it.first, it.second))
+			n++;
+	}
+	infostream << "Copied " << n << " files directly from server to client cache"
+		<< std::endl;
+}
+
 bool Game::createClient(const GameStartData &start_data)
 {
 	showOverlayMessage(N_("Creating client..."), 0, 10);

src/filesys.cpp

@@ -33,6 +33,13 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include <IFileArchive.h>
 #include <IFileSystem.h>
 #endif
+#ifdef __linux__
+#include <fcntl.h>
+#include <sys/ioctl.h>
+#ifndef FICLONE
+#define FICLONE _IOW(0x94, 9, int)
+#endif
+#endif
 
 namespace fs
 {
@@ -216,6 +223,31 @@ std::string CreateTempFile()
 	return path;
 }
 
+bool CopyFileContents(const std::string &source, const std::string &target)
+{
+	BOOL ok = CopyFileEx(source.c_str(), target.c_str(), nullptr, nullptr,
+		nullptr, COPY_FILE_ALLOW_DECRYPTED_DESTINATION);
+	if (!ok) {
+		errorstream << "copying " << source << " to " << target
+			<< " failed: " << GetLastError() << std::endl;
+		return false;
+	}
+
+	// docs: "File attributes for the existing file are copied to the new file."
+	// This is not our intention so get rid of unwanted attributes:
+	DWORD attr = GetFileAttributes(target.c_str());
+	if (attr == INVALID_FILE_ATTRIBUTES) {
+		errorstream << target << ": file disappeared after copy" << std::endl;
+		return false;
+	}
+	attr &= ~(FILE_ATTRIBUTE_READONLY | FILE_ATTRIBUTE_HIDDEN);
+	SetFileAttributes(target.c_str(), attr);
+
+	tracestream << "copied " << source << " to " << target
+		<< " using CopyFileEx" << std::endl;
+	return true;
+}
+
 #else
 
 /*********
@@ -414,6 +446,101 @@ std::string CreateTempFile()
 	return path;
 }
 
+namespace {
+	struct FileDeleter {
+		void operator()(FILE *stream) {
+			fclose(stream);
+		}
+	};
+
+	typedef std::unique_ptr<FILE, FileDeleter> FileUniquePtr;
+}
+
+bool CopyFileContents(const std::string &source, const std::string &target)
+{
+	FileUniquePtr sourcefile, targetfile;
+
+#ifdef __linux__
+	// Try to clone using Copy-on-Write (CoW). This is instant but supported
+	// only by some filesystems.
+
+	int srcfd, tgtfd;
+	srcfd = open(source.c_str(), O_RDONLY);
+	if (srcfd == -1) {
+		errorstream << source << ": can't open for reading: "
+			<< strerror(errno) << std::endl;
+		return false;
+	}
+	tgtfd = open(target.c_str(), O_WRONLY | O_CREAT | O_TRUNC, 0644);
+	if (tgtfd == -1) {
+		errorstream << target << ": can't open for writing: "
+			<< strerror(errno) << std::endl;
+		close(srcfd);
+		return false;
+	}
+
+	if (ioctl(tgtfd, FICLONE, srcfd) == 0) {
+		tracestream << "copied " << source << " to " << target
+			<< " using FICLONE" << std::endl;
+		close(srcfd);
+		close(tgtfd);
+		return true;
+	}
+
+	// fallback to normal copy, but no need to reopen the files
+	sourcefile.reset(fdopen(srcfd, "rb"));
+	targetfile.reset(fdopen(tgtfd, "wb"));
+	goto fallback;
+
+#endif
+
+	sourcefile.reset(fopen(source.c_str(), "rb"));
+	targetfile.reset(fopen(target.c_str(), "wb"));
+
+fallback:
+
+	if (!sourcefile) {
+		errorstream << source << ": can't open for reading: "
+			<< strerror(errno) << std::endl;
+		return false;
+	}
+	if (!targetfile) {
+		errorstream << target << ": can't open for writing: "
+			<< strerror(errno) << std::endl;
+		return false;
+	}
+
+	size_t total = 0;
+	bool done = false;
+	char readbuffer[BUFSIZ];
+	while (!done) {
+		size_t readbytes = fread(readbuffer, 1,
+				sizeof(readbuffer), sourcefile.get());
+		total += readbytes;
+		if (ferror(sourcefile.get())) {
+			errorstream << source << ": IO error: "
+				<< strerror(errno) << std::endl;
+			return false;
+		}
+		if (readbytes > 0)
+			fwrite(readbuffer, 1, readbytes, targetfile.get());
+		if (feof(sourcefile.get())) {
+			// flush destination file to catch write errors (e.g. disk full)
+			fflush(targetfile.get());
+			done = true;
+		}
+		if (ferror(targetfile.get())) {
+			errorstream << target << ": IO error: "
+					<< strerror(errno) << std::endl;
+			return false;
+		}
+	}
+	tracestream << "copied " << total << " bytes from "
+		<< source << " to " << target << std::endl;
+
+	return true;
+}
+
 #endif
 
 /****************************
@@ -488,60 +615,6 @@ bool CreateAllDirs(const std::string &path)
 	return true;
 }
 
-bool CopyFileContents(const std::string &source, const std::string &target)
-{
-	FILE *sourcefile = fopen(source.c_str(), "rb");
-	if(sourcefile == NULL){
-		errorstream<<source<<": can't open for reading: "
-			<<strerror(errno)<<std::endl;
-		return false;
-	}
-
-	FILE *targetfile = fopen(target.c_str(), "wb");
-	if(targetfile == NULL){
-		errorstream<<target<<": can't open for writing: "
-			<<strerror(errno)<<std::endl;
-		fclose(sourcefile);
-		return false;
-	}
-
-	size_t total = 0;
-	bool retval = true;
-	bool done = false;
-	char readbuffer[BUFSIZ];
-	while(!done){
-		size_t readbytes = fread(readbuffer, 1,
-				sizeof(readbuffer), sourcefile);
-		total += readbytes;
-		if(ferror(sourcefile)){
-			errorstream<<source<<": IO error: "
-				<<strerror(errno)<<std::endl;
-			retval = false;
-			done = true;
-		}
-		if(readbytes > 0){
-			fwrite(readbuffer, 1, readbytes, targetfile);
-		}
-		if(feof(sourcefile) || ferror(sourcefile)){
-			// flush destination file to catch write errors
-			// (e.g. disk full)
-			fflush(targetfile);
-			done = true;
-		}
-		if(ferror(targetfile)){
-			errorstream<<target<<": IO error: "
-					<<strerror(errno)<<std::endl;
-			retval = false;
-			done = true;
-		}
-	}
-	infostream<<"copied "<<total<<" bytes from "
-		<<source<<" to "<<target<<std::endl;
-	fclose(sourcefile);
-	fclose(targetfile);
-	return retval;
-}
-
 bool CopyDir(const std::string &source, const std::string &target)
 {
 	if(PathExists(source)){

src/server.cpp

@@ -4108,6 +4108,19 @@ Translations *Server::getTranslationLanguage(const std::string &lang_code)
 	return translations;
 }
 
+std::unordered_map<std::string, std::string> Server::getMediaList()
+{
+	MutexAutoLock env_lock(m_env_mutex);
+
+	std::unordered_map<std::string, std::string> ret;
+	for (auto &it : m_media) {
+		if (it.second.no_announce)
+			continue;
+		ret.emplace(base64_decode(it.second.sha1_digest), it.second.path);
+	}
+	return ret;
+}
+
 ModStorageDatabase *Server::openModStorageDatabase(const std::string &world_path)
 {
 	std::string world_mt_path = world_path + DIR_DELIM + "world.mt";

src/server.h

@@ -388,6 +388,10 @@ public:
 	// Get or load translations for a language
 	Translations *getTranslationLanguage(const std::string &lang_code);
 
+	// Returns all media files the server knows about
+	// map key = binary sha1, map value = file path
+	std::unordered_map<std::string, std::string> getMediaList();
+
 	static ModStorageDatabase *openModStorageDatabase(const std::string &world_path);
 
 	static ModStorageDatabase *openModStorageDatabase(const std::string &backend,

src/serverenvironment.cpp

@@ -1044,7 +1044,8 @@ void ServerEnvironment::activateBlock(MapBlock *block, u32 additional_dtime)
 			<<stamp<<", game time: "<<m_game_time<<std::endl;*/
 
 	// Remove stored static objects if clearObjects was called since block's timestamp
-	if (stamp == BLOCK_TIMESTAMP_UNDEFINED || stamp < m_last_clear_objects_time) {
+	// Note that non-generated blocks may still have stored static objects
+	if (stamp != BLOCK_TIMESTAMP_UNDEFINED && stamp < m_last_clear_objects_time) {
 		block->m_static_objects.clearStored();
 		// do not set changed flag to avoid unnecessary mapblock writes
 	}
@@ -1892,11 +1893,6 @@ u16 ServerEnvironment::addActiveObjectRaw(std::unique_ptr<ServerActiveObject> ob
 		return 0;
 	}
 
-	// Register reference in scripting api (must be done before post-init)
-	m_script->addObjectReference(object);
-	// Post-initialize object
-	object->addedToEnvironment(dtime_s);
-
 	// Add static data to block
 	if (object->isStaticAllowed()) {
 		// Add static object to active static list of the block
@@ -1915,12 +1911,20 @@ u16 ServerEnvironment::addActiveObjectRaw(std::unique_ptr<ServerActiveObject> ob
 					MOD_REASON_ADD_ACTIVE_OBJECT_RAW);
 		} else {
 			v3s16 p = floatToInt(objectpos, BS);
-			errorstream<<"ServerEnvironment::addActiveObjectRaw(): "
-				<<"could not emerge block for storing id="<<object->getId()
-				<<" statically (pos="<<p<<")"<<std::endl;
+			errorstream << "ServerEnvironment::addActiveObjectRaw(): "
+				<< "could not emerge block " << p << " for storing id="
+				<< object->getId() << " statically" << std::endl;
+			// clean in case of error
+			m_ao_manager.removeObject(object->getId());
+			return 0;
 		}
 	}
 
+	// Register reference in scripting api (must be done before post-init)
+	m_script->addObjectReference(object);
+	// Post-initialize object
+	object->addedToEnvironment(dtime_s);
+
 	return object->getId();
 }
 

src/unittest/test_datastructures.cpp

@@ -108,6 +108,8 @@ void TestDataStructures::testMap1()
 			break;
 	}
 	UASSERT(once);
+
+	map.clear(); // ASan complains about stack-use-after-scope otherwise
 }
 
 void TestDataStructures::testMap2()
@@ -121,6 +123,8 @@ void TestDataStructures::testMap2()
 	UASSERT(t0.deleted);
 	UASSERT(!t1.copied);
 	UASSERT(!t1.deleted);
+
+	map.clear();
 }
 
 void TestDataStructures::testMap3()

src/unittest/test_filesys.cpp

@@ -40,6 +40,7 @@ public:
 	void testRemoveLastPathComponentWithTrailingDelimiter();
 	void testRemoveRelativePathComponent();
 	void testSafeWriteToFile();
+	void testCopyFileContents();
 };
 
 static TestFileSys g_test_instance;
@@ -52,6 +53,7 @@ void TestFileSys::runTests(IGameDef *gamedef)
 	TEST(testRemoveLastPathComponentWithTrailingDelimiter);
 	TEST(testRemoveRelativePathComponent);
 	TEST(testSafeWriteToFile);
+	TEST(testCopyFileContents);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -59,7 +61,7 @@ void TestFileSys::runTests(IGameDef *gamedef)
 // adjusts a POSIX path to system-specific conventions
 // -> changes '/' to DIR_DELIM
 // -> absolute paths start with "C:\\" on windows
-std::string p(std::string path)
+static std::string p(std::string path)
 {
 	for (size_t i = 0; i < path.size(); ++i) {
 		if (path[i] == '/') {
@@ -275,5 +277,37 @@ void TestFileSys::testSafeWriteToFile()
 	UASSERT(fs::PathExists(dest_path));
 	std::string contents_actual;
 	UASSERT(fs::ReadFile(dest_path, contents_actual));
-	UASSERT(contents_actual == test_data);
+	UASSERTEQ(auto, contents_actual, test_data);
+}
+
+void TestFileSys::testCopyFileContents()
+{
+	const auto dir_path = getTestTempDirectory();
+	const auto file1 = dir_path + DIR_DELIM "src", file2 = dir_path + DIR_DELIM "dst";
+	const std::string test_data("hello\0world", 11);
+
+	// error case
+	UASSERT(!fs::CopyFileContents(file1, "somewhere"));
+
+	{
+		std::ofstream ofs(file1);
+		ofs << test_data;
+	}
+
+	// normal case
+	UASSERT(fs::CopyFileContents(file1, file2));
+	std::string contents_actual;
+	UASSERT(fs::ReadFile(file2, contents_actual));
+	UASSERTEQ(auto, contents_actual, test_data);
+
+	// should overwrite and truncate
+	{
+		std::ofstream ofs(file2);
+		for (int i = 0; i < 10; i++)
+			ofs << "OH MY GAH";
+	}
+	UASSERT(fs::CopyFileContents(file1, file2));
+	contents_actual.clear();
+	UASSERT(fs::ReadFile(file2, contents_actual));
+	UASSERTEQ(auto, contents_actual, test_data);
 }

util/wireshark/minetest.lua

@@ -1280,13 +1280,14 @@ do
 
 	function p_minetest.dissector(buffer, pinfo, tree)
 
-		-- Add Minetest tree item and verify the ID
+		-- Defer if payload doesn't have Minetest's magic number
+		if buffer(0,4):uint() ~= minetest_id then
+			return false
+		end
+
+		-- Add Minetest tree item
 		local t = tree:add(p_minetest, buffer(0,8))
 		t:add(f_id, buffer(0,4))
-		if buffer(0,4):uint() ~= minetest_id then
-			t:add_expert_info(PI_UNDECODED, PI_WARN, "Invalid ID, this is not a Minetest packet")
-			return
-		end
 
 		-- ID is valid, so replace packet's shown protocol
 		pinfo.cols.protocol = "Minetest"
@@ -1339,12 +1340,11 @@ do
 		end
 
 		pinfo.cols.info:append(" (" .. reliability_info .. ")")
+		return true
 
 	end
 
-	-- FIXME Is there a way to let the dissector table check if the first payload bytes are 0x4f457403?
-	DissectorTable.get("udp.port"):add(30000, p_minetest)
-	DissectorTable.get("udp.port"):add(30001, p_minetest)
+	p_minetest:register_heuristic("udp", p_minetest.dissector)
 end