Fix A LOT of type problems, logic flaws, uninitialized variables etc.

Most of them were found by phpstorm, so I put in some time and
went through the list, fixing quite a bunch of them.

11 files changed, 130 insertions, 52 deletions

diff --git a/inc/dashboard.inc.php b/inc/dashboard.inc.php
index 2cf0b494..3db42efb 100644
--- a/inc/dashboard.inc.php
+++ b/inc/dashboard.inc.php
@@ -41,8 +41,8 @@ class Dashboard
 		}
 		$currentPage = Page::getModule()->getIdentifier();
 		$categories = array();
-		$catSort = array();
 		foreach ($modByCategory as $catId => $modList) {
+			/* @var Module[] $modList */
 			$modules = array();
 			foreach ($modList as $modId => $module) {
 				if ($module === false)
diff --git a/inc/dictionary.inc.php b/inc/dictionary.inc.php
index ca2811ff..ee196f59 100644
--- a/inc/dictionary.inc.php
+++ b/inc/dictionary.inc.php
@@ -3,7 +3,13 @@
 class Dictionary
 {
 
+	/**
+	 * @var string[] Array of languages, numeric index, two letter CC as values
+	 */
 	private static $languages = false;
+	/**
+	 * @var array Array of languages, numeric index, values are ['name' => 'Language Name', 'cc' => 'xx']
+	 */
 	private static $languagesLong = false;
 	private static $stringCache = array();
 
@@ -49,16 +55,24 @@ class Dictionary
 		define('LANG', $language);
 	}
 
-	public static function getArray($module, $path, $lang = false)
+	/**
+	 * Get complete key=>value list for given module, file, language
+	 *
+	 * @param string $module Module name
+	 * @param string $file Dictionary name
+	 * @param string|false $lang Language CC, false === current language
+	 * @return array assoc array mapping language tags to the translated strings
+	 */
+	public static function getArray($module, $file, $lang = false)
 	{
 		if ($lang === false)
 			$lang = LANG;
-		$file = Util::safePath("modules/{$module}/lang/{$lang}/{$path}.json");
-		if (isset(self::$stringCache[$file]))
-			return self::$stringCache[$file];
-		if (!file_exists($file))
+		$path = Util::safePath("modules/{$module}/lang/{$lang}/{$file}.json");
+		if (isset(self::$stringCache[$path]))
+			return self::$stringCache[$path];
+		if (!file_exists($path))
 			return array();
-		$content = file_get_contents($file);
+		$content = file_get_contents($path);
 		if ($content === false) { // File does not exist for language
 			$content = '[]';
 		}
@@ -66,12 +80,22 @@ class Dictionary
 		if (!is_array($json)) {
 			$json = array();
 		}
-		return self::$stringCache[$file] = $json;
+		return self::$stringCache[$path] = $json;
 	}
 
-	public static function translateFileModule($moduleId, $path, $tag, $returnTagOnMissing = false)
+	/**
+	 * Translate a tag from a dictionary of a module. The current
+	 * language will be used.
+	 *
+	 * @param string $moduleId The module in question
+	 * @param string $file Dictionary name
+	 * @param string $tag Tag name
+	 * @param bool $returnTagOnMissing If true, the tag name enclosed in {{}} will be returned if the tag does not exist
+	 * @return string|false The requested tag's translation, or false if not found and $returnTagOnMissing === false
+	 */
+	public static function translateFileModule($moduleId, $file, $tag, $returnTagOnMissing = false)
 	{
-		$strings = self::getArray($moduleId, $path);
+		$strings = self::getArray($moduleId, $file);
 		if (!isset($strings[$tag])) {
 			if ($returnTagOnMissing) {
 				return '{{' . $tag . '}}';
@@ -80,14 +104,29 @@ class Dictionary
 		}
 		return $strings[$tag];
 	}
-	
-	public static function translateFile($path, $tag, $returnTagOnMissing = false)
+
+	/**
+	 * Translate a tag from a dictionary of the current module, using the current language.
+	 *
+	 * @param string $file Dictionary name
+	 * @param string $tag Tag name
+	 * @param bool $returnTagOnMissing If true, the tag name enclosed in {{}} will be returned if the tag does not exist
+	 * @return string|false The requested tag's translation, or false if not found and $returnTagOnMissing === false
+	 */
+	public static function translateFile($file, $tag, $returnTagOnMissing = false)
 	{
 		if (!class_exists('Page') || Page::getModule() === false)
 			return false; // We have no page - return false for now, as we're most likely running in api or install mode
-		return self::translateFileModule(Page::getModule()->getIdentifier(), $path, $tag, $returnTagOnMissing);
+		return self::translateFileModule(Page::getModule()->getIdentifier(), $file, $tag, $returnTagOnMissing);
 	}
 
+	/**
+	 * Translate a tag from the current module's default dictionary, using the current language.
+	 *
+	 * @param string $tag Tag name
+	 * @param bool $returnTagOnMissing If true, the tag name enclosed in {{}} will be returned if the tag does not exist
+	 * @return string|false The requested tag's translation, or false if not found and $returnTagOnMissing === false
+	 */
 	public static function translate($tag, $returnTagOnMissing = false)
 	{
 		$string = self::translateFile('module', $tag);
@@ -99,6 +138,13 @@ class Dictionary
 		return '{{' . $tag . '}}';
 	}
 
+	/**
+	 * Translate the given message id, reading the given module's messages dictionary.
+	 *
+	 * @param string $module Module the message belongs to
+	 * @param string $id Message id
+	 * @return string|false
+	 */
 	public static function getMessage($module, $id)
 	{
 		$string = self::translateFileModule($module, 'messages', $id);
@@ -107,7 +153,13 @@ class Dictionary
 		}
 		return $string;
 	}
-	
+
+	/**
+	 * Get translation of the given category.
+	 *
+	 * @param string $category
+	 * @return string Category name, or some generic fallback to the given category id
+	 */
 	public static function getCategoryName($category)
 	{
 		if ($category === false) {
@@ -126,7 +178,7 @@ class Dictionary
 	/**
 	 * Get all supported languages as array.
 	 *
-	 * @param boolean $withName true = return assoc array containinc cc and name of all languages;
+	 * @param boolean $withName true = return assoc array containing cc and name of all languages;
 	 *		false = regular array containing only the ccs
 	 * @return array List of languages
 	 */
@@ -151,7 +203,14 @@ class Dictionary
 		}
 		return self::$languagesLong;
 	}
-	
+
+	/**
+	 * Get name of language matching given language CC.
+	 * Default to the CC if the language isn't known.
+	 *
+	 * @param string $langCC
+	 * @return string
+	 */
 	public static function getLanguageName($langCC)
 	{
 		if (file_exists("lang/$langCC/name.txt")) {
diff --git a/inc/download.inc.php b/inc/download.inc.php
index e5764d37..39f8e2e2 100644
--- a/inc/download.inc.php
+++ b/inc/download.inc.php
@@ -69,7 +69,7 @@ class Download
 	 * POST-Download file, obey given timeout in seconds
 	 * Return data on success, false on failure
 	 * @param string $url URL to fetch
-	 * @param array $params POST params to set in body, list of key-value-pairs
+	 * @param array|false $params POST params to set in body, list of key-value-pairs
 	 * @param int $timeout timeout in seconds
 	 * @param int $code HTTP response code, or 999 on error
 	 */
diff --git a/inc/module.inc.php b/inc/module.inc.php
index ec3d095b..7610c720 100644
--- a/inc/module.inc.php
+++ b/inc/module.inc.php
@@ -214,7 +214,7 @@ class Module
 	private function getDepsInternal(&$deps)
 	{
 		if (!is_array($this->dependencies))
-			return array();
+			return;
 		foreach ($this->dependencies as $dep) {
 			if (isset($deps[$dep])) // Handle cyclic dependencies
 				continue;
diff --git a/inc/property.inc.php b/inc/property.inc.php
index 0b4ea7b3..56adb823 100644
--- a/inc/property.inc.php
+++ b/inc/property.inc.php
@@ -243,7 +243,7 @@ class Property
 	
 	public static function setPasswordFieldType($value)
 	{
-		return self::set('password-type', $value);
+		self::set('password-type', $value);
 	}
 	
 	public static function getPasswordFieldType()
diff --git a/inc/render.inc.php b/inc/render.inc.php
index d09b2a8e..0ce39dbe 100644
--- a/inc/render.inc.php
+++ b/inc/render.inc.php
@@ -15,6 +15,9 @@ Render::init();
 class Render
 {
 
+	/**
+	 * @var Mustache_Engine
+	 */
 	private static $mustache = false;
 	private static $body = '';
 	private static $header = '';
@@ -81,7 +84,7 @@ class Render
 	';
 		// Include any module specific styles
 		foreach ($modules as $module) {
-			$files = $module->getCss($module != $pageModule);
+			$files = $module->getCss($module !== $pageModule);
 			foreach ($files as $file) {
 				echo '<link href="', $module->getDir(), '/', $file, '" rel="stylesheet" media="screen">';
 			}
@@ -110,7 +113,7 @@ class Render
 		<script src="script/slx-fixes.js"></script>
 	';
 		foreach ($modules as $module) {
-			$files = $module->getScripts($module != $pageModule);
+			$files = $module->getScripts($module !== $pageModule);
 			foreach ($files as $file) {
 				echo '<script src="', $module->getDir(), '/', $file, '"></script>';
 			}
diff --git a/inc/taskmanager.inc.php b/inc/taskmanager.inc.php
index dcf54448..cdc90f55 100644
--- a/inc/taskmanager.inc.php
+++ b/inc/taskmanager.inc.php
@@ -29,7 +29,7 @@ class Taskmanager
 	 * @param array $data data to pass to the task. the structure depends on the task.
 	 * @param boolean $async if true, the function will not wait for the reply of the taskmanager, which means
 	 * 		the return value is just true (and you won't know if the task could acutally be started)
-	 * @return array struct representing the task status (as a result of submit); false on communication error
+	 * @return array|false struct representing the task status (as a result of submit); false on communication error
 	 */
 	public static function submit($task, $data = false, $async = false)
 	{
@@ -60,7 +60,7 @@ class Taskmanager
 	 * Query status of given task.
 	 *
 	 * @param mixed $task task id or task struct
-	 * @return array status of task as array, or false on communication error
+	 * @return array|false status of task as array, or false on communication error
 	 */
 	public static function status($task)
 	{
@@ -72,7 +72,7 @@ class Taskmanager
 		self::init();
 		$seq = (string) mt_rand();
 		$message = "$seq,     status, $task";
-		$sent = socket_send(self::$sock, $message, strlen($message), 0);
+		socket_send(self::$sock, $message, strlen($message), 0);
 		$reply = self::readReply($seq);
 		if (!is_array($reply))
 			return false;
@@ -96,9 +96,9 @@ class Taskmanager
 	/**
 	 * Wait for the given task's completion.
 	 *
-	 * @param array $task task to wait for
+	 * @param string|array $task task to wait for
 	 * @param int $timeout maximum time in ms to wait for completion of task
-	 * @return array result/status of task, or false if it couldn't be queried
+	 * @return array|false result/status of task, or false if it couldn't be queried
 	 */
 	public static function waitComplete($task, $timeout = 2500)
 	{
@@ -112,7 +112,8 @@ class Taskmanager
 		if (!is_string($task))
 			return false;
 		$done = false;
-		for ($i = 0; $i < ($timeout / 150); ++$i) {
+		$deadline = microtime(true) + $timeout / 1000;
+		do {
 			$status = self::status($task);
 			if (!isset($status['statusCode']))
 				break;
@@ -121,9 +122,10 @@ class Taskmanager
 				break;
 			}
 			usleep(100000);
-		}
-		if ($done)
+		} while (microtime(true) < $deadline);
+		if ($done) { // For now we do this unconditionally, but maybe we want to keep them longer some time?
 			self::release($task);
+		}
 		return $status;
 	}
 
@@ -131,7 +133,7 @@ class Taskmanager
 	 * Check whether the given task can be considered failed. This
 	 * includes that the task id is invalid, etc.
 	 *
-	 * @param array $task struct representing task, obtained by ::status
+	 * @param array|false $task struct representing task, obtained by ::status
 	 * @return boolean true if task failed, false if finished successfully or still waiting/running
 	 */
 	public static function isFailed($task)
@@ -183,7 +185,7 @@ class Taskmanager
 	/**
 	 * Release a given task from the task manager, so it won't keep the result anymore in case it's finished running.
 	 *
-	 * @param string $task task to release. can either be its id, or a struct representing the task, as returned
+	 * @param string|array $task task to release. can either be its id, or a struct representing the task, as returned
 	 * by ::submit() or ::status()
 	 */
 	public static function release($task)
diff --git a/inc/trigger.inc.php b/inc/trigger.inc.php
index db4a2148..2af73872 100644
--- a/inc/trigger.inc.php
+++ b/inc/trigger.inc.php
@@ -111,7 +111,7 @@ class Trigger
 	 * Mount the VM store into the server.
 	 *
 	 * @param array $vmstore VM Store configuration to use. If false, read from properties
-	 * @return array task status of mount procedure, or false on error
+	 * @return array|false task status of mount procedure, or false on error
 	 */
 	public static function mount($vmstore = false)
 	{
@@ -125,12 +125,13 @@ class Trigger
 		} else {
 			$storetype = 'unknown';
 		}
-		if ($storetype === 'nfs')
+		if ($storetype === 'nfs') {
 			$addr = $vmstore['nfsaddr'];
-		if ($storetype === 'cifs')
+		} elseif ($storetype === 'cifs') {
 			$addr = $vmstore['cifsaddr'];
-		if ($storetype === 'internal')
+		} else {
 			$addr = 'null';
+		}
 		return Taskmanager::submit('MountVmStore', array(
 				'address' => $addr,
 				'type' => 'images',
@@ -175,6 +176,16 @@ class Trigger
 			$taskids['dmsdid'] = $task['id'];
 			$parent = $task['id'];
 		}
+		$task = Taskmanager::submit('Systemctl', array(
+			'operation' => $action,
+			'service' => 'dnbd3-server',
+			'parentTask' => $parent,
+			'failOnParentFail' => false
+		));
+		if (isset($task['id'])) {
+			$taskids['dnbd3id'] = $task['id'];
+			$parent = $task['id'];
+		}
 		return $parent;
 	}
 
diff --git a/inc/up_json_encode.php b/inc/up_json_encode.php
index 0f5776c2..ac47ef51 100644
--- a/inc/up_json_encode.php
+++ b/inc/up_json_encode.php
@@ -90,7 +90,6 @@ function up_json_encode($var, $options = 0, $_indent = "")
 
 	#-- prepare JSON string
 	list($_space, $_tab, $_nl) = ($options & JSON_PRETTY_PRINT) ? array(" ", "    $_indent", "\n") : array("", "", "");
-	$json = "$_indent";
 
 	if (($options & JSON_NUMERIC_CHECK) and is_string($var) and is_numeric($var)) {
 		$var = (strpos($var, ".") || strpos($var, "e")) ? floatval($var) : intval($var);
@@ -143,7 +142,6 @@ function up_json_encode($var, $options = 0, $_indent = "")
 			"<" => $options & JSON_HEX_TAG ? "\\u003C" : "<",
 			">" => $options & JSON_HEX_TAG ? "\\u003E" : ">",
 			"'" => $options & JSON_HEX_APOS ? "\\u0027" : "'",
-			"\"" => "\\u0022",
 			"&" => $options & JSON_HEX_AMP ? "\\u0026" : "&",
 		);
 		$var = strtr($var, $rewrite);
diff --git a/inc/user.inc.php b/inc/user.inc.php
index 1e6a2d0e..95680e65 100644
--- a/inc/user.inc.php
+++ b/inc/user.inc.php
@@ -79,7 +79,7 @@ class User
 	public static function updatePassword($password)
 	{
 		if (!self::isLoggedIn())
-			return;
+			return false;
 		$passwd = Crypto::hash6($password);
 		$userid = self::getId();
 		return Database::exec('UPDATE user SET passwd = :passwd WHERE userid = :userid LIMIT 1', compact('userid', 'passwd')) > 0;
diff --git a/inc/util.inc.php b/inc/util.inc.php
index ace879f4..69eaf941 100644
--- a/inc/util.inc.php
+++ b/inc/util.inc.php
@@ -109,7 +109,7 @@ SADFACE;
 		return $arg;
 	}
 
-	public static function formatBacktraceHtml($trace, $escape = true)
+	public static function formatBacktraceHtml($trace)
 	{
 		$output = '';
 		foreach ($trace as $idx => $line) {
@@ -185,7 +185,7 @@ SADFACE;
 	
 	public static function addRedirectParam($key, $value)
 	{
-		self::$redirectParams[] = $key .= '=' . urlencode($value);
+		self::$redirectParams[] = $key . '=' . urlencode($value);
 	}
 
 	/**
@@ -225,14 +225,14 @@ SADFACE;
 	 * Convert given number to human readable file size string.
 	 * Will append Bytes, KiB, etc. depending on magnitude of number.
 	 * 
-	 * @param type $bytes numeric value of the filesize to make readable
-	 * @param type $decimals number of decimals to show, -1 for automatic
-	 * @return type human readable string representing the given filesize
+	 * @param float|int $bytes numeric value of the filesize to make readable
+	 * @param int $decimals number of decimals to show, -1 for automatic
+	 * @return string human readable string representing the given filesize
 	 */
 	public static function readableFileSize($bytes, $decimals = -1)
 	{
 		static $sz = array('Byte', 'KiB', 'MiB', 'GiB', 'TiB', 'PiB');
-		$factor = floor((strlen($bytes) - 1) / 3);
+		$factor = (int)floor((strlen($bytes) - 1) / 3);
 		if ($factor == 0) {
 			$decimals = 0;
 		} elseif ($decimals === -1) {
@@ -351,9 +351,9 @@ SADFACE;
 	/**
 	 * Send a file to user for download.
 	 *
-	 * @param type $file path of local file
-	 * @param type $name name of file to send to user agent
-	 * @param type $delete delete the file when done?
+	 * @param string $file path of local file
+	 * @param string $name name of file to send to user agent
+	 * @param boolean $delete delete the file when done?
 	 * @return boolean false: file could not be opened.
 	 *		true: error while reading the file
 	 *		- on success, the function does not return
@@ -362,7 +362,7 @@ SADFACE;
 	{
 		while ((@ob_get_level()) > 0)
 			@ob_end_clean();
-		$fh = @fopen($file, 'rb');
+		$fh = fopen($file, 'rb');
 		if ($fh === false) {
 			Message::addError('main.error-read', $file);
 			return false;
@@ -382,9 +382,10 @@ SADFACE;
 			@ob_flush();
 			@flush();
 		}
-		@fclose($fh);
-		if ($delete)
-			@unlink($file);
+		fclose($fh);
+		if ($delete) {
+			unlink($file);
+		}
 		exit(0);
 	}
 
@@ -401,7 +402,11 @@ SADFACE;
 	public static function randomBytes($length, $secure = true)
 	{
 		if (function_exists('random_bytes')) {
-			return random_bytes($length);
+			try {
+				return random_bytes($length);
+			} catch (Exception $e) {
+				// Continue below
+			}
 		}
 		if ($secure) {
 			if (function_exists('openssl_random_pseudo_bytes')) {