Reduce indentation of HTTPFetchOngoing

Also clean up some related things.

src/clientmedia.cpp

@@ -140,17 +140,17 @@ void ClientMediaDownloader::step(Client *client)
 	// Remote media: check for completion of fetches
 	if (m_httpfetch_active) {
 		bool fetched_something = false;
-		HTTPFetchResult fetchresult;
+		HTTPFetchResult fetch_result;
 
-		while (httpfetch_async_get(m_httpfetch_caller, fetchresult)) {
+		while (httpfetch_async_get(m_httpfetch_caller, fetch_result)) {
 			m_httpfetch_active--;
 			fetched_something = true;
 
 			// Is this a hashset (index.mth) or a media file?
-			if (fetchresult.request_id < m_remotes.size())
+			if (fetch_result.request_id < m_remotes.size())
-				remoteHashSetReceived(fetchresult);
+				remoteHashSetReceived(fetch_result);
 			else
-				remoteMediaReceived(fetchresult, client);
+				remoteMediaReceived(fetch_result, client);
 		}
 
 		if (fetched_something)
@@ -259,17 +259,17 @@ void ClientMediaDownloader::initialStep(Client *client)
 			actionstream << "Client: Contacting remote server \""
 				<< remote->baseurl << "\"" << std::endl;
 
-			HTTPFetchRequest fetchrequest;
+			HTTPFetchRequest fetch_request;
-			fetchrequest.url =
+			fetch_request.url =
 				remote->baseurl + MTHASHSET_FILE_NAME;
-			fetchrequest.caller = m_httpfetch_caller;
+			fetch_request.caller = m_httpfetch_caller;
-			fetchrequest.request_id = m_httpfetch_next_id; // == i
+			fetch_request.request_id = m_httpfetch_next_id; // == i
-			fetchrequest.timeout = m_httpfetch_timeout;
+			fetch_request.timeout = m_httpfetch_timeout;
-			fetchrequest.connect_timeout = m_httpfetch_timeout;
+			fetch_request.connect_timeout = m_httpfetch_timeout;
-			fetchrequest.post_data = required_hash_set;
+			fetch_request.post_data = required_hash_set;
-			fetchrequest.extra_headers.push_back(
+			fetch_request.extra_headers.push_back(
 				"Content-Type: application/octet-stream");
-			httpfetch_async(fetchrequest);
+			httpfetch_async(fetch_request);
 
 			m_httpfetch_active++;
 			m_httpfetch_next_id++;
@@ -279,21 +279,21 @@ void ClientMediaDownloader::initialStep(Client *client)
 }
 
 void ClientMediaDownloader::remoteHashSetReceived(
-		const HTTPFetchResult &fetchresult)
+		const HTTPFetchResult &fetch_result)
 {
-	u32 remote_id = fetchresult.request_id;
+	u32 remote_id = fetch_result.request_id;
 	assert(remote_id < m_remotes.size());
 	RemoteServerStatus *remote = m_remotes[remote_id];
 
 	m_outstanding_hash_sets--;
 
-	if (fetchresult.succeeded) {
+	if (fetch_result.succeeded) {
 		try {
 			// Server sent a list of file hashes that are
 			// available on it, try to parse the list
 
 			std::set<std::string> sha1_set;
-			deSerializeHashSet(fetchresult.data, sha1_set);
+			deSerializeHashSet(fetch_result.data, sha1_set);
 
 			// Parsing succeeded: For every file that is
 			// available on this server, add this server
@@ -320,7 +320,7 @@ void ClientMediaDownloader::remoteHashSetReceived(
 	// Do NOT check for any particular response code (e.g. 404) here,
 	// because different servers respond differently
 
-	if (!fetchresult.succeeded && !fetchresult.timeout) {
+	if (!fetch_result.succeeded && !fetch_result.timeout) {
 		infostream << "Client: Enabling compatibility mode for remote "
 			<< "server \"" << remote->baseurl << "\"" << std::endl;
 		remote->request_by_filename = true;
@@ -338,7 +338,7 @@ void ClientMediaDownloader::remoteHashSetReceived(
 }
 
 void ClientMediaDownloader::remoteMediaReceived(
-		const HTTPFetchResult &fetchresult,
+		const HTTPFetchResult &fetch_result,
 		Client *client)
 {
 	// Some remote server sent us a file.
@@ -349,7 +349,7 @@ void ClientMediaDownloader::remoteMediaReceived(
 	std::string name;
 	{
 		std::map<unsigned long, std::string>::iterator it =
-			m_remote_file_transfers.find(fetchresult.request_id);
+			m_remote_file_transfers.find(fetch_result.request_id);
 		assert(it != m_remote_file_transfers.end());
 		name = it->second;
 		m_remote_file_transfers.erase(it);
@@ -368,9 +368,9 @@ void ClientMediaDownloader::remoteMediaReceived(
 
 	// If fetch succeeded, try to load media file
 
-	if (fetchresult.succeeded) {
+	if (fetch_result.succeeded) {
 		bool success = checkAndLoad(name, filestatus->sha1,
-				fetchresult.data, false, client);
+				fetch_result.data, false, client);
 		if (success) {
 			filestatus->received = true;
 			assert(m_uncached_received_count < m_uncached_count);
@@ -445,14 +445,14 @@ void ClientMediaDownloader::startRemoteMediaTransfers()
 					<< "\"" << name << "\" "
 					<< "\"" << url << "\"" << std::endl;
 
-				HTTPFetchRequest fetchrequest;
+				HTTPFetchRequest fetch_request;
-				fetchrequest.url = url;
+				fetch_request.url = url;
-				fetchrequest.caller = m_httpfetch_caller;
+				fetch_request.caller = m_httpfetch_caller;
-				fetchrequest.request_id = m_httpfetch_next_id;
+				fetch_request.request_id = m_httpfetch_next_id;
-				fetchrequest.timeout = 0; // no data timeout!
+				fetch_request.timeout = 0; // no data timeout!
-				fetchrequest.connect_timeout =
+				fetch_request.connect_timeout =
 					m_httpfetch_timeout;
-				httpfetch_async(fetchrequest);
+				httpfetch_async(fetch_request);
 
 				m_remote_file_transfers.insert(std::make_pair(
 							m_httpfetch_next_id,

src/clientmedia.h

@@ -96,8 +96,8 @@ private:
 	};
 
 	void initialStep(Client *client);
-	void remoteHashSetReceived(const HTTPFetchResult &fetchresult);
+	void remoteHashSetReceived(const HTTPFetchResult &fetch_result);
-	void remoteMediaReceived(const HTTPFetchResult &fetchresult,
+	void remoteMediaReceived(const HTTPFetchResult &fetch_result,
 			Client *client);
 	s32 selectRemoteServer(FileStatus *filestatus);
 	void startRemoteMediaTransfers();

src/convert_json.cpp

@@ -30,45 +30,34 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "httpfetch.h"
 #include "porting.h"
 
-Json::Value                 fetchJsonValue(const std::string &url,
+Json::Value fetchJsonValue(const std::string &url,
-		std::vector<std::string> *extra_headers) {
+		std::vector<std::string> *extra_headers)
-
+{
-	HTTPFetchRequest fetchrequest;
+	HTTPFetchRequest fetch_request;
-	HTTPFetchResult fetchresult;
+	HTTPFetchResult fetch_result;
-	fetchrequest.url = url;
+	fetch_request.url = url;
-	fetchrequest.caller = HTTPFETCH_SYNC;
+	fetch_request.caller = HTTPFETCH_SYNC;
 
 	if (extra_headers != NULL)
-		fetchrequest.extra_headers = *extra_headers;
+		fetch_request.extra_headers = *extra_headers;
 
-	httpfetch_sync(fetchrequest,fetchresult);
+	httpfetch_sync(fetch_request, fetch_result);
 
-	if (!fetchresult.succeeded) {
+	if (!fetch_result.succeeded) {
 		return Json::Value();
 	}
 	Json::Value root;
 	Json::Reader reader;
-	std::istringstream stream(fetchresult.data);
+	std::istringstream stream(fetch_result.data);
 
-	if (!reader.parse( stream, root ) )
+	if (!reader.parse(stream, root)) {
-	{
 		errorstream << "URL: " << url << std::endl;
 		errorstream << "Failed to parse json data " << reader.getFormattedErrorMessages();
-		errorstream << "data: \"" << fetchresult.data << "\"" << std::endl;
+		errorstream << "data: \"" << fetch_result.data << "\"" << std::endl;
 		return Json::Value();
 	}
 
-	if (root.isArray()) {
+	return root;
-		return root;
-	}
-	if ((root["list"].isArray())) {
-		return root["list"];
-	}
-	else {
-		return root;
-	}
-
-	return Json::Value();
 }
 
 std::vector<ModStoreMod>    readModStoreList(Json::Value& modlist) {

src/guiEngine.cpp

@@ -528,27 +528,26 @@ bool GUIEngine::setTexture(texture_layer layer, std::string texturepath,
 }
 
 /******************************************************************************/
-bool GUIEngine::downloadFile(std::string url,std::string target)
+bool GUIEngine::downloadFile(std::string url, std::string target)
 {
 #if USE_CURL
-	std::ofstream targetfile(target.c_str(), std::ios::out | std::ios::binary);
+	std::ofstream target_file(target.c_str(), std::ios::out | std::ios::binary);
 
-	if (!targetfile.good()) {
+	if (!target_file.good()) {
 		return false;
 	}
 
-	HTTPFetchRequest fetchrequest;
+	HTTPFetchRequest fetch_request;
-	HTTPFetchResult fetchresult;
+	HTTPFetchResult fetch_result;
-	fetchrequest.url = url;
+	fetch_request.url = url;
-	fetchrequest.caller = HTTPFETCH_SYNC;
+	fetch_request.caller = HTTPFETCH_SYNC;
-	fetchrequest.timeout = g_settings->getS32("curl_file_download_timeout");
+	fetch_request.timeout = g_settings->getS32("curl_file_download_timeout");
-	httpfetch_sync(fetchrequest, fetchresult);
+	httpfetch_sync(fetch_request, fetch_result);
 
-	if (fetchresult.succeeded) {
+	if (!fetch_result.succeeded) {
-		targetfile << fetchresult.data;
-	} else {
 		return false;
 	}
+	target_file << fetch_result.data;
 
 	return true;
 #else

src/httpfetch.cpp

@@ -52,12 +52,12 @@ HTTPFetchRequest::HTTPFetchRequest()
 }
 
 
-static void httpfetch_deliver_result(const HTTPFetchResult &fetchresult)
+static void httpfetch_deliver_result(const HTTPFetchResult &fetch_result)
 {
-	unsigned long caller = fetchresult.caller;
+	unsigned long caller = fetch_result.caller;
 	if (caller != HTTPFETCH_DISCARD) {
 		JMutexAutoLock lock(g_httpfetch_mutex);
-		g_httpfetch_results[caller].push_back(fetchresult);
+		g_httpfetch_results[caller].push_back(fetch_result);
 	}
 }
 
@@ -97,7 +97,7 @@ void httpfetch_caller_free(unsigned long caller)
 	}
 }
 
-bool httpfetch_async_get(unsigned long caller, HTTPFetchResult &fetchresult)
+bool httpfetch_async_get(unsigned long caller, HTTPFetchResult &fetch_result)
 {
 	JMutexAutoLock lock(g_httpfetch_mutex);
 
@@ -108,13 +108,13 @@ bool httpfetch_async_get(unsigned long caller, HTTPFetchResult &fetchresult)
 		return false;
 
 	// Check that result queue is nonempty
-	std::list<HTTPFetchResult> &callerresults = it->second;
+	std::list<HTTPFetchResult> &caller_results = it->second;
-	if (callerresults.empty())
+	if (caller_results.empty())
 		return false;
 
 	// Pop first result
-	fetchresult = callerresults.front();
+	fetch_result = caller_results.front();
-	callerresults.pop_front();
+	caller_results.pop_front();
 	return true;
 }
 
@@ -175,8 +175,19 @@ public:
 	}
 };
 
-struct HTTPFetchOngoing
+class HTTPFetchOngoing
 {
+public:
+	HTTPFetchOngoing(HTTPFetchRequest request, CurlHandlePool *pool);
+	~HTTPFetchOngoing();
+
+	CURLcode start(CURLM *multi);
+	const HTTPFetchResult * complete(CURLcode res);
+
+	const HTTPFetchRequest &getRequest()    const { return request; };
+	const CURL             *getEasyHandle() const { return curl; };
+
+private:
 	CurlHandlePool *pool;
 	CURL *curl;
 	CURLM *multi;
@@ -184,201 +195,200 @@ struct HTTPFetchOngoing
 	HTTPFetchResult result;
 	std::ostringstream oss;
 	char *post_fields;
-	struct curl_slist *httpheader;
+	struct curl_slist *http_header;
 	curl_httppost *post;
+};
 
-	HTTPFetchOngoing(HTTPFetchRequest request_, CurlHandlePool *pool_):
+
-		pool(pool_),
+HTTPFetchOngoing::HTTPFetchOngoing(HTTPFetchRequest request_, CurlHandlePool *pool_):
-		curl(NULL),
+	pool(pool_),
-		multi(NULL),
+	curl(NULL),
-		request(request_),
+	multi(NULL),
-		result(request_),
+	request(request_),
-		oss(std::ios::binary),
+	result(request_),
-		httpheader(NULL),
+	oss(std::ios::binary),
-		post(NULL)
+	http_header(NULL),
-	{
+	post(NULL)
-		curl = pool->alloc();
+{
-		if (curl != NULL) {
+	curl = pool->alloc();
-			// Set static cURL options
+	if (curl == NULL) {
-			curl_easy_setopt(curl, CURLOPT_NOSIGNAL, 1);
+		return;
-			curl_easy_setopt(curl, CURLOPT_FAILONERROR, 1);
+	}
-			curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1);
+
-			curl_easy_setopt(curl, CURLOPT_MAXREDIRS, 1);
+	// Set static cURL options
+	curl_easy_setopt(curl, CURLOPT_NOSIGNAL, 1);
+	curl_easy_setopt(curl, CURLOPT_FAILONERROR, 1);
+	curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1);
+	curl_easy_setopt(curl, CURLOPT_MAXREDIRS, 1);
 
 #if LIBCURL_VERSION_NUM >= 0x071304
-			// Restrict protocols so that curl vulnerabilities in
+	// Restrict protocols so that curl vulnerabilities in
-			// other protocols don't affect us.
+	// other protocols don't affect us.
-			// These settings were introduced in curl 7.19.4.
+	// These settings were introduced in curl 7.19.4.
-			long protocols =
+	long protocols =
-				CURLPROTO_HTTP |
+		CURLPROTO_HTTP |
-				CURLPROTO_HTTPS |
+		CURLPROTO_HTTPS |
-				CURLPROTO_FTP |
+		CURLPROTO_FTP |
-				CURLPROTO_FTPS;
+		CURLPROTO_FTPS;
-			curl_easy_setopt(curl, CURLOPT_PROTOCOLS, protocols);
+	curl_easy_setopt(curl, CURLOPT_PROTOCOLS, protocols);
-			curl_easy_setopt(curl, CURLOPT_REDIR_PROTOCOLS, protocols);
+	curl_easy_setopt(curl, CURLOPT_REDIR_PROTOCOLS, protocols);
 #endif
 
-			// Set cURL options based on HTTPFetchRequest
+	// Set cURL options based on HTTPFetchRequest
-			curl_easy_setopt(curl, CURLOPT_URL,
+	curl_easy_setopt(curl, CURLOPT_URL,
-					request.url.c_str());
+			request.url.c_str());
-			curl_easy_setopt(curl, CURLOPT_TIMEOUT_MS,
+	curl_easy_setopt(curl, CURLOPT_TIMEOUT_MS,
-					request.timeout);
+			request.timeout);
-			curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT_MS,
+	curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT_MS,
-					request.connect_timeout);
+			request.connect_timeout);
 
-			if (request.useragent != "")
+	if (request.useragent != "")
-				curl_easy_setopt(curl, CURLOPT_USERAGENT, request.useragent.c_str());
+		curl_easy_setopt(curl, CURLOPT_USERAGENT, request.useragent.c_str());
 
-			// Set up a write callback that writes to the
+	// Set up a write callback that writes to the
-			// ostringstream ongoing->oss, unless the data
+	// ostringstream ongoing->oss, unless the data
-			// is to be discarded
+	// is to be discarded
-			if (request.caller == HTTPFETCH_DISCARD) {
+	if (request.caller == HTTPFETCH_DISCARD) {
-				curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION,
-						httpfetch_discardfunction);
-				curl_easy_setopt(curl, CURLOPT_WRITEDATA, NULL);
-			}
-			else {
-				curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION,
-						httpfetch_writefunction);
-				curl_easy_setopt(curl, CURLOPT_WRITEDATA, &oss);
-			}
-
-			// Set POST (or GET) data
-			if (request.post_fields.empty()) {
-				curl_easy_setopt(curl, CURLOPT_HTTPGET, 1);
-			} else if (request.multipart) {
-				curl_httppost *last = NULL;
-				for (std::map<std::string, std::string>::iterator it =
-							request.post_fields.begin();
-						it != request.post_fields.end();
-						++it) {
-					curl_formadd(&post, &last,
-							CURLFORM_NAMELENGTH, it->first.size(),
-							CURLFORM_PTRNAME, it->first.c_str(),
-							CURLFORM_CONTENTSLENGTH, it->second.size(),
-							CURLFORM_PTRCONTENTS, it->second.c_str(),
-							CURLFORM_END);
-				}
-				curl_easy_setopt(curl, CURLOPT_HTTPPOST, post);
-				// request.post_fields must now *never* be
-				// modified until CURLOPT_HTTPPOST is cleared
-			} else {
-				curl_easy_setopt(curl, CURLOPT_POST, 1);
-				if (request.post_data.empty()) {
-					std::string str;
-					for (std::map<std::string, std::string>::iterator it =
-								request.post_fields.begin();
-							it != request.post_fields.end();
-							++it) {
-						if (str != "")
-							str += "&";
-						str += urlencode(it->first);
-						str += "=";
-						str += urlencode(it->second);
-					}
-					curl_easy_setopt(curl, CURLOPT_POSTFIELDSIZE,
-							str.size());
-					curl_easy_setopt(curl, CURLOPT_COPYPOSTFIELDS,
-							str.c_str());
-				} else {
-					curl_easy_setopt(curl, CURLOPT_POSTFIELDSIZE,
-							request.post_data.size());
-					curl_easy_setopt(curl, CURLOPT_POSTFIELDS,
-							request.post_data.c_str());
-					// request.post_data must now *never* be
-					// modified until CURLOPT_POSTFIELDS is cleared
-				}
-			}
-			// Set additional HTTP headers
-			for (size_t i = 0; i < request.extra_headers.size(); ++i) {
-				httpheader = curl_slist_append(
-					httpheader,
-					request.extra_headers[i].c_str());
-			}
-			curl_easy_setopt(curl, CURLOPT_HTTPHEADER, httpheader);
-
-			if (!g_settings->getBool("curl_verify_cert")) {
-				curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, false);
-			}
-		}
-	}
-
-	CURLcode start(CURLM *multi_)
-	{
-		if (curl == NULL)
-			return CURLE_FAILED_INIT;
-
-		if (multi_) {
-			// Multi interface (async)
-			CURLMcode mres = curl_multi_add_handle(multi_, curl);
-			if (mres != CURLM_OK) {
-				errorstream<<"curl_multi_add_handle"
-					<<" returned error code "<<mres
-					<<std::endl;
-				return CURLE_FAILED_INIT;
-			}
-			multi = multi_; // store for curl_multi_remove_handle
-			return CURLE_OK;
-		}
-		else {
-			// Easy interface (sync)
-			return curl_easy_perform(curl);
-		}
-	}
-
-	void complete(CURLcode res)
-	{
-		result.succeeded = (res == CURLE_OK);
-		result.timeout = (res == CURLE_OPERATION_TIMEDOUT);
-		result.data = oss.str();
-
-		// Get HTTP/FTP response code
-		result.response_code = 0;
-		if (curl != NULL) {
-			if (curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE,
-					&result.response_code) != CURLE_OK) {
-				//we failed to get a return code make sure it is still 0
-				result.response_code = 0;
-			}
-		}
-
-		if (res != CURLE_OK) {
-			errorstream<<request.url<<" not found ("
-				<<curl_easy_strerror(res)<<")"
-				<<" (response code "<<result.response_code<<")"
-				<<std::endl;
-		}
-	}
-
-	~HTTPFetchOngoing()
-	{
-		if (multi != NULL) {
-			CURLMcode mres = curl_multi_remove_handle(multi, curl);
-			if (mres != CURLM_OK) {
-				errorstream<<"curl_multi_remove_handle"
-					<<" returned error code "<<mres
-					<<std::endl;
-			}
-		}
-
-		// Set safe options for the reusable cURL handle
 		curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION,
 				httpfetch_discardfunction);
 		curl_easy_setopt(curl, CURLOPT_WRITEDATA, NULL);
-		curl_easy_setopt(curl, CURLOPT_POSTFIELDS, NULL);
+	} else {
-		if (httpheader != NULL) {
+		curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION,
-			curl_easy_setopt(curl, CURLOPT_HTTPHEADER, NULL);
+				httpfetch_writefunction);
-			curl_slist_free_all(httpheader);
+		curl_easy_setopt(curl, CURLOPT_WRITEDATA, &oss);
-		}
-		if (post != NULL) {
-			curl_easy_setopt(curl, CURLOPT_HTTPPOST, NULL);
-			curl_formfree(post);
-		}
-
-		// Store the cURL handle for reuse
-		pool->free(curl);
 	}
-};
+
+	// Set POST (or GET) data
+	if (request.post_fields.empty()) {
+		curl_easy_setopt(curl, CURLOPT_HTTPGET, 1);
+	} else if (request.multipart) {
+		curl_httppost *last = NULL;
+		for (std::map<std::string, std::string>::iterator it =
+					request.post_fields.begin();
+				it != request.post_fields.end(); ++it) {
+			curl_formadd(&post, &last,
+					CURLFORM_NAMELENGTH, it->first.size(),
+					CURLFORM_PTRNAME, it->first.c_str(),
+					CURLFORM_CONTENTSLENGTH, it->second.size(),
+					CURLFORM_PTRCONTENTS, it->second.c_str(),
+					CURLFORM_END);
+		}
+		curl_easy_setopt(curl, CURLOPT_HTTPPOST, post);
+		// request.post_fields must now *never* be
+		// modified until CURLOPT_HTTPPOST is cleared
+	} else if (request.post_data.empty()) {
+		curl_easy_setopt(curl, CURLOPT_POST, 1);
+		std::string str;
+		for (std::map<std::string, std::string>::iterator it =
+					request.post_fields.begin();
+				it != request.post_fields.end();
+				++it) {
+			if (str != "")
+				str += "&";
+			str += urlencode(it->first);
+			str += "=";
+			str += urlencode(it->second);
+		}
+		curl_easy_setopt(curl, CURLOPT_POSTFIELDSIZE,
+				str.size());
+		curl_easy_setopt(curl, CURLOPT_COPYPOSTFIELDS,
+				str.c_str());
+	} else {
+		curl_easy_setopt(curl, CURLOPT_POST, 1);
+		curl_easy_setopt(curl, CURLOPT_POSTFIELDSIZE,
+				request.post_data.size());
+		curl_easy_setopt(curl, CURLOPT_POSTFIELDS,
+				request.post_data.c_str());
+		// request.post_data must now *never* be
+		// modified until CURLOPT_POSTFIELDS is cleared
+	}
+	// Set additional HTTP headers
+	for (std::vector<std::string>::iterator it = request.extra_headers.begin();
+			it != request.extra_headers.end(); ++it) {
+		http_header = curl_slist_append(http_header, it->c_str());
+	}
+	curl_easy_setopt(curl, CURLOPT_HTTPHEADER, http_header);
+
+	if (!g_settings->getBool("curl_verify_cert")) {
+		curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, false);
+	}
+}
+
+CURLcode HTTPFetchOngoing::start(CURLM *multi_)
+{
+	if (!curl)
+		return CURLE_FAILED_INIT;
+
+	if (!multi_) {
+		// Easy interface (sync)
+		return curl_easy_perform(curl);
+	}
+
+	// Multi interface (async)
+	CURLMcode mres = curl_multi_add_handle(multi_, curl);
+	if (mres != CURLM_OK) {
+		errorstream << "curl_multi_add_handle"
+			<< " returned error code " << mres
+			<< std::endl;
+		return CURLE_FAILED_INIT;
+	}
+	multi = multi_; // store for curl_multi_remove_handle
+	return CURLE_OK;
+}
+
+const HTTPFetchResult * HTTPFetchOngoing::complete(CURLcode res)
+{
+	result.succeeded = (res == CURLE_OK);
+	result.timeout = (res == CURLE_OPERATION_TIMEDOUT);
+	result.data = oss.str();
+
+	// Get HTTP/FTP response code
+	result.response_code = 0;
+	if (curl && (curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE,
+				&result.response_code) != CURLE_OK)) {
+		// We failed to get a return code, make sure it is still 0
+		result.response_code = 0;
+	}
+
+	if (res != CURLE_OK) {
+		errorstream << request.url << " not found ("
+			<< curl_easy_strerror(res) << ")"
+			<< " (response code " << result.response_code << ")"
+			<< std::endl;
+	}
+
+	return &result;
+}
+
+HTTPFetchOngoing::~HTTPFetchOngoing()
+{
+	if (multi) {
+		CURLMcode mres = curl_multi_remove_handle(multi, curl);
+		if (mres != CURLM_OK) {
+			errorstream << "curl_multi_remove_handle"
+				<< " returned error code " << mres
+				<< std::endl;
+		}
+	}
+
+	// Set safe options for the reusable cURL handle
+	curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION,
+			httpfetch_discardfunction);
+	curl_easy_setopt(curl, CURLOPT_WRITEDATA, NULL);
+	curl_easy_setopt(curl, CURLOPT_POSTFIELDS, NULL);
+	if (http_header) {
+		curl_easy_setopt(curl, CURLOPT_HTTPHEADER, NULL);
+		curl_slist_free_all(http_header);
+	}
+	if (post) {
+		curl_easy_setopt(curl, CURLOPT_HTTPPOST, NULL);
+		curl_formfree(post);
+	}
+
+	// Store the cURL handle for reuse
+	pool->free(curl);
+}
+
 
 class CurlFetchThread : public JThread
 {
@@ -391,7 +401,7 @@ protected:
 
 	struct Request {
 		RequestType type;
-		HTTPFetchRequest fetchrequest;
+		HTTPFetchRequest fetch_request;
 		Event *event;
 	};
 
@@ -412,11 +422,11 @@ public:
 			m_parallel_limit = 1;
 	}
 
-	void requestFetch(const HTTPFetchRequest &fetchrequest)
+	void requestFetch(const HTTPFetchRequest &fetch_request)
 	{
 		Request req;
 		req.type = RT_FETCH;
-		req.fetchrequest = fetchrequest;
+		req.fetch_request = fetch_request;
 		req.event = NULL;
 		m_requests.push_back(req);
 	}
@@ -425,7 +435,7 @@ public:
 	{
 		Request req;
 		req.type = RT_CLEAR;
-		req.fetchrequest.caller = caller;
+		req.fetch_request.caller = caller;
 		req.event = event;
 		m_requests.push_back(req);
 	}
@@ -446,24 +456,24 @@ protected:
 		if (req.type == RT_FETCH) {
 			// New fetch, queue until there are less
 			// than m_parallel_limit ongoing fetches
-			m_queued_fetches.push_back(req.fetchrequest);
+			m_queued_fetches.push_back(req.fetch_request);
 
 			// see processQueued() for what happens next
 
 		}
 		else if (req.type == RT_CLEAR) {
-			unsigned long caller = req.fetchrequest.caller;
+			unsigned long caller = req.fetch_request.caller;
 
 			// Abort all ongoing fetches for the caller
 			for (std::vector<HTTPFetchOngoing*>::iterator
 					it = m_all_ongoing.begin();
 					it != m_all_ongoing.end();) {
-				if ((*it)->request.caller == caller) {
+				if ((*it)->getRequest().caller == caller) {
 					delete (*it);
 					it = m_all_ongoing.erase(it);
-				}
+				} else {
-				else
 					++it;
+				}
 			}
 
 			// Also abort all queued fetches for the caller
@@ -503,8 +513,7 @@ protected:
 				m_all_ongoing.push_back(ongoing);
 			}
 			else {
-				ongoing->complete(res);
+				httpfetch_deliver_result(*ongoing->complete(res));
-				httpfetch_deliver_result(ongoing->result);
 				delete ongoing;
 			}
 		}
@@ -517,7 +526,7 @@ protected:
 		size_t i = 0;
 		bool found = false;
 		for (i = 0; i < m_all_ongoing.size(); ++i) {
-			if (m_all_ongoing[i]->curl == msg->easy_handle) {
+			if (m_all_ongoing[i]->getEasyHandle() == msg->easy_handle) {
 				found = true;
 				break;
 			}
@@ -525,8 +534,7 @@ protected:
 		if (msg->msg == CURLMSG_DONE && found) {
 			// m_all_ongoing[i] succeeded or failed.
 			HTTPFetchOngoing *ongoing = m_all_ongoing[i];
-			ongoing->complete(msg->data.result);
+			httpfetch_deliver_result(*ongoing->complete(msg->data.result));
-			httpfetch_deliver_result(ongoing->result);
 			delete ongoing;
 			m_all_ongoing.erase(m_all_ongoing.begin() + i);
 		}
@@ -719,9 +727,9 @@ void httpfetch_cleanup()
 	curl_global_cleanup();
 }
 
-void httpfetch_async(const HTTPFetchRequest &fetchrequest)
+void httpfetch_async(const HTTPFetchRequest &fetch_request)
 {
-	g_httpfetch_thread->requestFetch(fetchrequest);
+	g_httpfetch_thread->requestFetch(fetch_request);
 	if (!g_httpfetch_thread->IsRunning())
 		g_httpfetch_thread->Start();
 }
@@ -738,18 +746,17 @@ static void httpfetch_request_clear(unsigned long caller)
 	}
 }
 
-void httpfetch_sync(const HTTPFetchRequest &fetchrequest,
+void httpfetch_sync(const HTTPFetchRequest &fetch_request,
-		HTTPFetchResult &fetchresult)
+		HTTPFetchResult &fetch_result)
 {
 	// Create ongoing fetch data and make a cURL handle
 	// Set cURL options based on HTTPFetchRequest
 	CurlHandlePool pool;
-	HTTPFetchOngoing ongoing(fetchrequest, &pool);
+	HTTPFetchOngoing ongoing(fetch_request, &pool);
 	// Do the fetch (curl_easy_perform)
 	CURLcode res = ongoing.start(NULL);
-	// Update fetchresult
+	// Update fetch result
-	ongoing.complete(res);
+	fetch_result = *ongoing.complete(res);
-	fetchresult = ongoing.result;
 }
 
 #else  // USE_CURL
@@ -768,26 +775,26 @@ void httpfetch_cleanup()
 {
 }
 
-void httpfetch_async(const HTTPFetchRequest &fetchrequest)
+void httpfetch_async(const HTTPFetchRequest &fetch_request)
 {
-	errorstream<<"httpfetch_async: unable to fetch "<<fetchrequest.url
+	errorstream << "httpfetch_async: unable to fetch " << fetch_request.url
-			<<" because USE_CURL=0"<<std::endl;
+			<< " because USE_CURL=0" << std::endl;
 
-	HTTPFetchResult fetchresult(fetchrequest); // sets succeeded = false etc.
+	HTTPFetchResult fetch_result(fetch_request); // sets succeeded = false etc.
-	httpfetch_deliver_result(fetchresult);
+	httpfetch_deliver_result(fetch_result);
 }
 
 static void httpfetch_request_clear(unsigned long caller)
 {
 }
 
-void httpfetch_sync(const HTTPFetchRequest &fetchrequest,
+void httpfetch_sync(const HTTPFetchRequest &fetch_request,
-		HTTPFetchResult &fetchresult)
+		HTTPFetchResult &fetch_result)
 {
-	errorstream<<"httpfetch_sync: unable to fetch "<<fetchrequest.url
+	errorstream << "httpfetch_sync: unable to fetch " << fetch_request.url
-			<<" because USE_CURL=0"<<std::endl;
+			<< " because USE_CURL=0" << std::endl;
 
-	fetchresult = HTTPFetchResult(fetchrequest); // sets succeeded = false etc.
+	fetch_result = HTTPFetchResult(fetch_request); // sets succeeded = false etc.
 }
 
 #endif  // USE_CURL

src/httpfetch.h

@@ -88,14 +88,14 @@ struct HTTPFetchResult
 		request_id = 0;
 	}
 
-	HTTPFetchResult(const HTTPFetchRequest &fetchrequest)
+	HTTPFetchResult(const HTTPFetchRequest &fetch_request)
 	{
 		succeeded = false;
 		timeout = false;
 		response_code = 0;
 		data = "";
-		caller = fetchrequest.caller;
+		caller = fetch_request.caller;
-		request_id = fetchrequest.request_id;
+		request_id = fetch_request.request_id;
 	}
 
 };
@@ -107,11 +107,11 @@ void httpfetch_init(int parallel_limit);
 void httpfetch_cleanup();
 
 // Starts an asynchronous HTTP fetch request
-void httpfetch_async(const HTTPFetchRequest &fetchrequest);
+void httpfetch_async(const HTTPFetchRequest &fetch_request);
 
 // If any fetch for the given caller ID is complete, removes it from the
-// result queue, sets fetchresult and returns true. Otherwise returns false.
+// result queue, sets the fetch result and returns true. Otherwise returns false.
-bool httpfetch_async_get(unsigned long caller, HTTPFetchResult &fetchresult);
+bool httpfetch_async_get(unsigned long caller, HTTPFetchResult &fetch_result);
 
 // Allocates a caller ID for httpfetch_async
 // Not required if you want to set caller = HTTPFETCH_DISCARD
@@ -124,8 +124,8 @@ void httpfetch_caller_free(unsigned long caller);
 
 // Performs a synchronous HTTP request. This blocks and therefore should
 // only be used from background threads.
-void httpfetch_sync(const HTTPFetchRequest &fetchrequest,
+void httpfetch_sync(const HTTPFetchRequest &fetch_request,
-		HTTPFetchResult &fetchresult);
+		HTTPFetchResult &fetch_result);
 
 
 #endif // !HTTPFETCH_HEADER

src/mods.cpp

@@ -339,19 +339,14 @@ Json::Value getModstoreUrl(std::string url)
 
 	bool special_http_header = true;
 
-	try{
+	try {
 		special_http_header = g_settings->getBool("modstore_disable_special_http_header");
-	}
+	} catch (SettingNotFoundException) {}
-	catch(SettingNotFoundException &e) {
-	}
 
 	if (special_http_header) {
 		extra_headers.push_back("Accept: application/vnd.minetest.mmdb-v1+json");
-		return fetchJsonValue(url, &extra_headers);
-	}
-	else {
-		return fetchJsonValue(url, NULL);
 	}
+	return fetchJsonValue(url, special_http_header ? &extra_headers : NULL);
 }
 
 #endif

src/serverlist.cpp

@@ -70,17 +70,24 @@ std::vector<ServerListSpec> getOnline()
 	Json::Value root = fetchJsonValue(
 			(g_settings->get("serverlist_url") + "/list").c_str(), NULL);
 
-	std::vector<ServerListSpec> serverlist;
+	std::vector<ServerListSpec> server_list;
 
-	if (root.isArray()) {
+	if (!root.isObject()) {
-		for (unsigned int i = 0; i < root.size(); i++) {
+		return server_list;
-			if (root[i].isObject()) {
+	}
-				serverlist.push_back(root[i]);
+
-			}
+	root = root["list"];
+	if (!root.isArray()) {
+		return server_list;
+	}
+
+	for (unsigned int i = 0; i < root.size(); i++) {
+		if (root[i].isObject()) {
+			server_list.push_back(root[i]);
 		}
 	}
 
-	return serverlist;
+	return server_list;
 }
 
 
@@ -236,11 +243,11 @@ void sendAnnounce(const std::string &action,
 	}
 
 	Json::FastWriter writer;
-	HTTPFetchRequest fetchrequest;
+	HTTPFetchRequest fetch_request;
-	fetchrequest.url = g_settings->get("serverlist_url") + std::string("/announce");
+	fetch_request.url = g_settings->get("serverlist_url") + std::string("/announce");
-	fetchrequest.post_fields["json"] = writer.write(server);
+	fetch_request.post_fields["json"] = writer.write(server);
-	fetchrequest.multipart = true;
+	fetch_request.multipart = true;
-	httpfetch_async(fetchrequest);
+	httpfetch_async(fetch_request);
 }
 #endif