core: use C++ std::strings for default directory and filename

The memory management and string concatenation was hard to follow.

Since the mobile files ios.cpp and android.cpp were already
converted to C++, let's do the same for Unix, Windows and MacOS.

Simply store the default directory and filename in a function-level
static string. Thus, it will be initialized on first call and
freed on application exit. Since the std::string data is
guaranteed to be contiguous and zero-terminated, it can be used
from C code.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>

core/CMakeLists.txt

@@ -4,16 +4,16 @@ if(CMAKE_SYSTEM_NAME STREQUAL "Linux")
 	if(ANDROID)
 		set(PLATFORM_SRC android.cpp serial_usb_android.cpp)
 	else()
-		set(PLATFORM_SRC unix.c)
+		set(PLATFORM_SRC unix.cpp)
 	endif()
 elseif(CMAKE_SYSTEM_NAME STREQUAL "Android")
 	set(PLATFORM_SRC android.cpp serial_usb_android.cpp)
 elseif(CMAKE_SYSTEM_NAME STREQUAL "Darwin")
-	set(PLATFORM_SRC macos.c)
+	set(PLATFORM_SRC macos.cpp)
 elseif(CMAKE_SYSTEM_NAME STREQUAL "Windows")
-	set(PLATFORM_SRC windows.c)
+	set(PLATFORM_SRC windows.cpp)
 elseif(CMAKE_SYSTEM_NAME STREQUAL "OpenBSD")
-	set(PLATFORM_SRC unix.c)
+	set(PLATFORM_SRC unix.cpp)
 endif()
 
 if(FTDISUPPORT)

core/android.cpp

@@ -12,6 +12,7 @@
 #include <errno.h>
 #include <unistd.h>
 #include <zip.h>
+#include <string>
 
 #include <QtAndroidExtras/QtAndroidExtras>
 #include <QtAndroidExtras/QAndroidJniObject>
@@ -26,9 +27,20 @@
 #define LOG(x) qDebug() << x;
 #endif
 
-
 #define USB_SERVICE "usb"
 
+static std::string system_default_path()
+{
+	// Qt appears to find a working path for us - let's just go with that
+	// AppDataLocation allows potential sharing of the files we put there
+	return QStandardPaths::standardLocations(QStandardPaths::AppDataLocation).first().toStdString();
+}
+
+static std::string make_default_filename()
+{
+	return system_default_path() + "/subsurface.xml";
+}
+
 extern "C" {
 
 const char android_system_divelist_default_font[] = "Roboto";
@@ -46,35 +58,19 @@ bool subsurface_ignore_font(const char *font)
 	return false;
 }
 
-static const char *system_default_path_append(const char *append)
-{
-	// Qt appears to find a working path for us - let's just go with that
-	// AppDataLocation allows potential sharing of the files we put there
-	QString path = QStandardPaths::standardLocations(QStandardPaths::AppDataLocation).first();
-
-	if (append)
-		path += QString("/%1").arg(append);
-
-	return copy_qstring(path);
-}
-
 const char *system_default_directory(void)
 {
-	static const char *path = NULL;
+	static const std::string path = system_default_path();
-	if (!path)
+	return path.c_str();
-		path = system_default_path_append(NULL);
-	return path;
 }
 
 const char *system_default_filename(void)
 {
-	static const char *filename = "subsurface.xml";
+	static const std::string fn = make_default_filename();
-	static const char *path = NULL;
+	return fn.c_str();
-	if (!path)
-		path = system_default_path_append(filename);
-	return path;
 }
 
+
 int enumerate_devices(device_callback_t callback, void *userdata, unsigned int transport)
 {
 	/* FIXME: we need to enumerate in some other way on android */

core/ios.cpp

@@ -17,9 +17,21 @@
 #include <fcntl.h>
 #include <unistd.h>
 #include <zip.h>
+#include <string>
 
 #include <QStandardPaths>
 
+static std::string system_default_path()
+{
+	// Qt appears to find a working path for us - let's just go with that
+	return QStandardPaths::standardLocations(QStandardPaths::DataLocation).first().toStdString();
+}
+
+static std::string make_default_filename()
+{
+	return system_default_path() + "/subsurface.xml";
+}
+
 extern "C" {
 
 const char mac_system_divelist_default_font[] = "Arial";
@@ -37,32 +49,16 @@ bool subsurface_ignore_font(const char*)
 	return false;
 }
 
-static const char *system_default_path_append(const char *append)
-{
-	// Qt appears to find a working path for us - let's just go with that
-	QString path = QStandardPaths::standardLocations(QStandardPaths::DataLocation).first();
-
-	if (append)
-		path += QString("/%1").arg(append);
-
-	return copy_qstring(path);
-}
-
 const char *system_default_directory(void)
 {
-	static const char *path = NULL;
+	static const std::string path = system_default_path();
-	if (!path)
+	return path.c_str();
-		path = system_default_path_append(NULL);
-	return path;
 }
 
 const char *system_default_filename(void)
 {
-	static const char *filename = "subsurface.xml";
+	static const std::string fn = make_default_filename();
-	static const char *path = NULL;
+	return fn.c_str();
-	if (!path)
-		path = system_default_path_append(filename);
-	return path;
 }
 
 int enumerate_devices(device_callback_t, void *, unsigned int)

core/macos.cpp

@@ -20,6 +20,7 @@
 #include <unistd.h>
 #include <zip.h>
 #include <sys/stat.h>
+#include <string>
 
 /* macos defines CFSTR to create a CFString object from a constant,
  * but no similar macros if a C string variable is supposed to be
@@ -33,6 +34,22 @@
 #define ICON_NAME "Subsurface.icns"
 #define UI_FONT "Arial 12"
 
+static std::string system_default_path()
+{
+	std::string home(getenv("HOME"));
+	return home + "/Library/Application Support/Subsurface";
+}
+
+static std::string make_default_filename()
+{
+	std::string user = getenv("LOGNAME");
+	if (user.empty())
+		user = "username";
+	return system_default_path() + "/" + user + ".xml";
+}
+
+extern "C" {
+
 const char mac_system_divelist_default_font[] = "Arial";
 const char *system_divelist_default_font = mac_system_divelist_default_font;
 double system_divelist_default_font_size = -1.0;
@@ -42,56 +59,22 @@ void subsurface_OS_pref_setup(void)
 	// nothing
 }
 
-bool subsurface_ignore_font(const char *font)
+bool subsurface_ignore_font(const char *)
 {
-	UNUSED(font);
 	// there are no old default fonts to ignore
 	return false;
 }
 
-static const char *system_default_path_append(const char *append)
-{
-	const char *home = getenv("HOME");
-	const char *path = "/Library/Application Support/Subsurface";
-
-	int len = strlen(home) + strlen(path) + 1;
-	if (append)
-		len += strlen(append) + 1;
-
-	char *buffer = (char *)malloc(len);
-	memset(buffer, 0, len);
-	strcat(buffer, home);
-	strcat(buffer, path);
-	if (append) {
-		strcat(buffer, "/");
-		strcat(buffer, append);
-	}
-
-	return buffer;
-}
-
 const char *system_default_directory(void)
 {
-	static const char *path = NULL;
+	static const std::string path = system_default_path();
-	if (!path)
+	return path.c_str();
-		path = system_default_path_append(NULL);
-	return path;
 }
 
 const char *system_default_filename(void)
 {
-	static const char *path = NULL;
+	static const std::string fn = make_default_filename();
-	if (!path) {
+	return fn.c_str();
-		const char *user = getenv("LOGNAME");
-		if (empty_string(user))
-			user = "username";
-		char *filename = calloc(strlen(user) + 5, 1);
-		strcat(filename, user);
-		strcat(filename, ".xml");
-		path = system_default_path_append(filename);
-		free(filename);
-	}
-	return path;
 }
 
 int enumerate_devices(device_callback_t callback, void *userdata, unsigned int transport)
@@ -223,3 +206,5 @@ bool subsurface_user_is_root()
 {
 	return geteuid() == 0;
 }
+
+}

core/unix.cpp

@@ -17,6 +17,25 @@
 #include <unistd.h>
 #include <pwd.h>
 #include <zip.h>
+#include <string>
+
+static std::string system_default_path()
+{
+	std::string home(getenv("HOME"));
+	if (home.empty())
+		home = "~";
+	return home + "/.subsurface";
+}
+
+static std::string make_default_filename()
+{
+	std::string user = getenv("LOGNAME");
+	if (user.empty())
+		user = "username";
+	return system_default_path() + "/" + user + ".xml";
+}
+
+extern "C" {
 
 // the DE should provide us with a default font and font size...
 const char unix_system_divelist_default_font[] = "Sans";
@@ -28,58 +47,22 @@ void subsurface_OS_pref_setup(void)
 	// nothing
 }
 
-bool subsurface_ignore_font(const char *font)
+bool subsurface_ignore_font(const char *)
 {
 	// there are no old default fonts to ignore
-	UNUSED(font);
 	return false;
 }
 
-static const char *system_default_path_append(const char *append)
-{
-	const char *home = getenv("HOME");
-	if (!home)
-		home = "~";
-	const char *path = "/.subsurface";
-
-	int len = strlen(home) + strlen(path) + 1;
-	if (append)
-		len += strlen(append) + 1;
-
-	char *buffer = (char *)malloc(len);
-	memset(buffer, 0, len);
-	strcat(buffer, home);
-	strcat(buffer, path);
-	if (append) {
-		strcat(buffer, "/");
-		strcat(buffer, append);
-	}
-
-	return buffer;
-}
-
 const char *system_default_directory(void)
 {
-	static const char *path = NULL;
+	static const std::string path = system_default_path();
-	if (!path)
+	return path.c_str();
-		path = system_default_path_append(NULL);
-	return path;
 }
 
 const char *system_default_filename(void)
 {
-	static const char *path = NULL;
+	static const std::string fn = make_default_filename();
-	if (!path) {
+	return fn.c_str();
-		const char *user = getenv("LOGNAME");
-		if (empty_string(user))
-			user = "username";
-		char *filename = calloc(strlen(user) + 5, 1);
-		strcat(filename, user);
-		strcat(filename, ".xml");
-		path = system_default_path_append(filename);
-		free(filename);
-	}
-	return path;
 }
 
 int enumerate_devices(device_callback_t callback, void *userdata, unsigned int transport)
@@ -228,3 +211,5 @@ bool subsurface_user_is_root()
 {
 	return geteuid() == 0;
 }
+
+}

core/windows.cpp

@@ -19,6 +19,71 @@
 #include <dirent.h>
 #include <zip.h>
 #include <lmcons.h>
+#include <string>
+
+/* this function converts a win32's utf-16 2 byte string to utf-8.
+ * note: the standard library's <codecvt> was deprecated and is in
+ * an ominous state, so use the native Windows version for now.
+ */
+static std::string utf16_to_utf8_fl(const std::wstring &utf16, char *file, int line)
+{
+	assert(utf16 != NULL);
+	assert(file != NULL);
+	assert(line);
+	/* estimate buffer size */
+	const int sz = WideCharToMultiByte(CP_UTF8, 0, utf16.c_str(), -1, NULL, 0, NULL, NULL);
+	if (!sz) {
+		fprintf(stderr, "%s:%d: cannot estimate buffer size\n", file, line);
+		return std::string();
+	}
+	std::string utf8(sz, ' '); // Note: includes the terminating '\0', just in case.
+	if (WideCharToMultiByte(CP_UTF8, 0, utf16.c_str(), -1, &utf8[0], sz, NULL, NULL)) {
+		utf8.resize(sz - 1); // Chop off final '\0' byte
+		return utf8;
+	}
+	fprintf(stderr, "%s:%d: cannot convert string\n", file, line);
+	return std::string();
+}
+
+#define utf16_to_utf8(s) utf16_to_utf8_fl(s, __FILE__, __LINE__)
+
+/* this function returns the Win32 Roaming path for the current user as UTF-8.
+ * it never returns an empty string but fallsback to .\ instead!
+ */
+static std::wstring system_default_path()
+{
+	wchar_t wpath[MAX_PATH] = { 0 };
+	const char *fname = "system_default_path()";
+
+	/* obtain the user path via SHGetFolderPathW.
+	 * this API is deprecated but still supported on modern Win32.
+	 * fallback to .\ if it fails.
+	 */
+	std::wstring path;
+	if (SUCCEEDED(SHGetFolderPathW(NULL, CSIDL_APPDATA, NULL, 0, wpath))) {
+		path = wpath;
+	} else {
+		fprintf(stderr, "%s: cannot obtain path!\n", fname);
+		path = L'.';
+	}
+	return path + L"\\Subsurface";
+}
+
+/* obtain the Roaming path and append "\\<USERNAME>.xml" to it.
+ */
+static std::wstring make_default_filename()
+{
+	wchar_t username[UNLEN + 1] = { 0 };
+	DWORD username_len = UNLEN + 1;
+	GetUserNameW(username, &username_len);
+	std::wstring filename = username;
+	filename += L".xml";
+
+	std::wstring path = system_default_path();
+	return path + L"\\" + filename;
+}
+
+extern "C" {
 
 const char non_standard_system_divelist_default_font[] = "Calibri";
 const char current_system_divelist_default_font[] = "Segoe UI";
@@ -40,35 +105,6 @@ bool subsurface_ignore_font(const char *font)
 	return false;
 }
 
-/* this function converts a win32's utf-16 2 byte string to utf-8.
- * the caller function should manage the allocated memory.
- */
-static char *utf16_to_utf8_fl(const wchar_t *utf16, char *file, int line)
-{
-	assert(utf16 != NULL);
-	assert(file != NULL);
-	assert(line);
-	/* estimate buffer size */
-	const int sz = WideCharToMultiByte(CP_UTF8, 0, utf16, -1, NULL, 0, NULL, NULL);
-	if (!sz) {
-		fprintf(stderr, "%s:%d: cannot estimate buffer size\n", file, line);
-		return NULL;
-	}
-	char *utf8 = (char *)malloc(sz);
-	if (!utf8) {
-		fprintf(stderr, "%s:%d: cannot allocate buffer of size: %d\n", file, line, sz);
-		return NULL;
-	}
-	if (WideCharToMultiByte(CP_UTF8, 0, utf16, -1, utf8, sz, NULL, NULL)) {
-		return utf8;
-	}
-	fprintf(stderr, "%s:%d: cannot convert string\n", file, line);
-	free((void *)utf8);
-	return NULL;
-}
-
-#define utf16_to_utf8(s) utf16_to_utf8_fl(s, __FILE__, __LINE__)
-
 /* this function converts a utf-8 string to win32's utf-16 2 byte string.
  * the caller function should manage the allocated memory.
  */
@@ -93,68 +129,18 @@ static wchar_t *utf8_to_utf16_fl(const char *utf8, char *file, int line)
 
 #define utf8_to_utf16(s) utf8_to_utf16_fl(s, __FILE__, __LINE__)
 
-/* this function returns the Win32 Roaming path for the current user as UTF-8.
+/* '\' not included at the end.
- * it never returns NULL but fallsback to .\ instead!
- * the append argument will append a wchar_t string to the end of the path.
- */
-static wchar_t *system_default_path_append(const wchar_t *append)
-{
-	wchar_t wpath[MAX_PATH] = { 0 };
-	const char *fname = "system_default_path_append()";
-
-	/* obtain the user path via SHGetFolderPathW.
-	 * this API is deprecated but still supported on modern Win32.
-	 * fallback to .\ if it fails.
-	 */
-	if (!SUCCEEDED(SHGetFolderPathW(NULL, CSIDL_APPDATA, NULL, 0, wpath))) {
-		fprintf(stderr, "%s: cannot obtain path!\n", fname);
-		wpath[0] = L'.';
-		wpath[1] = L'\0';
-	}
-
-	wcscat(wpath, L"\\Subsurface");
-	if (append) {
-		wcscat(wpath, L"\\");
-		wcscat(wpath, append);
-	}
-
-	wchar_t *result = wcsdup(wpath);
-	if (!result)
-		fprintf(stderr, "%s: cannot allocate memory for path!\n", fname);
-	return result;
-}
-
-/* by passing NULL to system_default_path_append() we obtain the pure path.
- * '\' not included at the end.
  */
 const char *system_default_directory(void)
 {
-	static const char *path = NULL;
+	static std::string path = utf16_to_utf8(system_default_path());
-	if (!path) {
+	return path.c_str();
-		wchar_t *wpath = system_default_path_append(NULL);
-		path = utf16_to_utf8(wpath);
-		free((void *)wpath);
-	}
-	return path;
 }
 
-/* obtain the Roaming path and append "\\<USERNAME>.xml" to it.
- */
 const char *system_default_filename(void)
 {
-	static const char *path = NULL;
+	static std::string path = utf16_to_utf8(make_default_filename());
-	if (!path) {
+	return path.c_str();
-		wchar_t username[UNLEN + 1] = { 0 };
-		DWORD username_len = UNLEN + 1;
-		GetUserNameW(username, &username_len);
-		wchar_t filename[UNLEN + 5] = { 0 };
-		wcscat(filename, username);
-		wcscat(filename, L".xml");
-		wchar_t *wpath = system_default_path_append(filename);
-		path = utf16_to_utf8(wpath);
-		free((void *)wpath);
-	}
-	return path;
 }
 
 int enumerate_devices(device_callback_t callback, void *userdata, unsigned int transport)
@@ -400,15 +386,16 @@ int subsurface_zip_close(struct zip *zip)
 }
 
 /* win32 console */
+#ifndef WIN32_CONSOLE_APP
 static struct {
 	bool allocated;
 	UINT cp;
 	FILE *out, *err;
 } console_desc;
+#endif
 
 void subsurface_console_init(void)
 {
-	UNUSED(console_desc);
 	/* if this is a console app already, do nothing */
 #ifndef WIN32_CONSOLE_APP
 
@@ -427,14 +414,11 @@ void subsurface_console_init(void)
 		console_desc.err = freopen("CON", "w", stderr);
 	} else {
 		verbose = 1; /* set the verbose level to '1' */
-		wchar_t *wpath_out = system_default_path_append(L"subsurface_out.log");
+		std::wstring path = system_default_path();
-		wchar_t *wpath_err = system_default_path_append(L"subsurface_err.log");
+		std::wstring wpath_out = path + L"\\subsurface_out.log";
-		if (wpath_out && wpath_err) {
+		std::wstring wpath_err = path + L"\\subsurface_err.log";
-			console_desc.out = _wfreopen(wpath_out, L"w", stdout);
+		console_desc.out = _wfreopen(wpath_out.c_str(), L"w", stdout);
-			console_desc.err = _wfreopen(wpath_err, L"w", stderr);
+		console_desc.err = _wfreopen(wpath_err.c_str(), L"w", stderr);
-		}
-		free((void *)wpath_out);
-		free((void *)wpath_err);
 	}
 
 	puts(""); /* add an empty line */
@@ -462,3 +446,5 @@ bool subsurface_user_is_root()
 	/* FIXME: Detect admin rights */
 	return false;
 }
+
+}

subsurface-desktop-main.cpp

@@ -47,7 +47,6 @@ int main(int argc, char **argv)
 	QStringList arguments = QCoreApplication::arguments();
 
 	const char *default_directory = system_default_directory();
-	const char *default_filename = system_default_filename();
 	subsurface_mkdir(default_directory);
 
 	for (i = 1; i < arguments.length(); i++) {
@@ -113,8 +112,6 @@ int main(int argc, char **argv)
 	clear_divelog(&divelog);
 	taglist_free(g_tag_list);
 	parse_xml_exit();
-	free((void *)default_directory);
-	free((void *)default_filename);
 	subsurface_console_exit();
 
 	// Sync struct preferences to disk

subsurface-downloader-main.cpp

@@ -47,7 +47,6 @@ int main(int argc, char **argv)
 	logfile_name = strdup("subsurface-downloader.log");
 
 	const char *default_directory = system_default_directory();
-	const char *default_filename = system_default_filename();
 	subsurface_mkdir(default_directory);
 
 	if (subsurface_user_is_root() && !force_root) {
@@ -107,8 +106,6 @@ int main(int argc, char **argv)
 	clear_divelog(&divelog);
 	taglist_free(g_tag_list);
 	parse_xml_exit();
-	free((void *)default_directory);
-	free((void *)default_filename);
 
 	// Sync struct preferences to disk
 	qPref::sync();