desktop: Tidy up a little again, add documentation

src/desktop/version.cpp

@@ -72,14 +72,33 @@ bool on_wine()
 #endif
 
 #if defined(_X11) || defined(__APPLE__)
+/**
+ * Release policy for POSIX pipe streams opened with popen(3).
+ */
 struct posix_pipe_release_policy
 {
 	void operator()(std::FILE* f) const { if(f != NULL) { pclose(f); } }
 };
 
+/**
+ * Scoped POSIX pipe stream.
+ *
+ * The stream object type is the same as a regular file stream, but the release
+ * policy is different, as required by popen(3).
+ */
 typedef util::scoped_resource<std::FILE*, posix_pipe_release_policy> scoped_posix_pipe;
 
-std::string read_version(scoped_posix_pipe& p) {
+/**
+ * Read a single line from the specified pipe.
+ *
+ * @returns An empty string if the pipe is invalid or nothing could be read.
+ */
+std::string read_pipe_line(scoped_posix_pipe& p)
+{
+	if(!p.get()) {
+		return "";
+	}
+
 	std::string ver;
 	int c;
 
@@ -101,6 +120,7 @@ std::string os_version()
 #if defined(_X11) || defined(__APPLE__)
 
 #ifdef __APPLE__
+
 	//
 	// Standard Mac OSX version
 	//
@@ -109,18 +129,18 @@ std::string os_version()
 	static const std::string defaults_bin = "/usr/bin/defaults";
 
 	if(filesystem::file_exists(defaults_bin) && filesystem::file_exists(version_plist)) {
-		static const std::string cmd_line = defaults_bin + " read " + version_plist + " ProductUserVisibleVersion";
-		scoped_posix_pipe p(popen(cmd_line.c_str(), "r"));
-
-		if(p.get()) {
-			const std::string& ver = read_version(p);
-
-			if(!ver.empty()) {
-				return "Mac OS X " + ver;
-			}
+		static const std::string cmdline
+				= defaults_bin + " read " + version_plist + " ProductUserVisibleVersion";
+
+		scoped_posix_pipe p(popen(cmdline.c_str(), "r"));
+		const std::string& ver = read_pipe_line(p);
+
+		if(!ver.empty()) {
+			return "Mac OS X " + ver;
 		}
 	}
-#endif
+
+#else
 
 	//
 	// Linux Standard Base version.
@@ -129,16 +149,16 @@ std::string os_version()
 	static const std::string lsb_release_bin = "/usr/bin/lsb_release";
 
 	if(filesystem::file_exists(lsb_release_bin)) {
-		scoped_posix_pipe p(popen((lsb_release_bin + " -s -d").c_str(), "r"));
+		static const std::string cmdline = lsb_release_bin + " -s -d";
 
-		if(p.get()) {
-			const std::string& ver = read_version(p);
-
-			if(!ver.empty()) {
-				return ver;
-			}
+		scoped_posix_pipe p(popen(cmdline.c_str(), "r"));
+		const std::string& ver = read_pipe_line(p);
+
+		if(!ver.empty()) {
+			return ver;
 		}
 	}
+#endif
 
 	//
 	// POSIX uname version.

src/desktop/version.hpp

@@ -28,12 +28,10 @@ namespace desktop
 /**
  * Returns a string with the running OS name and version information.
  *
- * On Unix-type platforms, this will be the uname information (which is rarely
- * useful). On Windows, this is a string we generate ourselves by processing
- * GetVersionEx's output data.
- *
- * Needless to say, this is a highly experimental and unreliable function and
- * odds are its output is not particularly useful most of the time.
+ * On Windows, this is a string we generate ourselves by processing
+ * GetVersionEx's output. On OS X and Linux, this is the output of a command
+ * provided by the OS if available; failing that (and on other Unixes as well),
+ * we use the uname system call, which is hardly ever useful.
  */
 std::string os_version();