Windows: build with UNICODE, get rid of all TCHAR/TEXT(), use explicit

xxA() interfaces and wchar_t in some places. Add a static cleanup retry
method to TempFile, called after clearing the MimeHandler cache (killing
the subprocesses which might hold an open file).

src/common/rclinit.cpp

@@ -205,16 +205,16 @@ LRESULT CALLBACK MainWndProc(HWND hwnd , UINT msg , WPARAM wParam,
 bool CreateInvisibleWindow()
 {
     HWND hwnd;
-    WNDCLASS wc = {0};
+    WNDCLASS wc = {0,0,0,0,0,0,0,0,0,0};
 
     wc.lpfnWndProc = (WNDPROC)MainWndProc;
     wc.hInstance = GetModuleHandle(NULL);
-    wc.hIcon = LoadIcon(GetModuleHandle(NULL), "TestWClass");
+    wc.hIcon = LoadIcon(GetModuleHandle(NULL), L"TestWClass");
-    wc.lpszClassName = "TestWClass";
+    wc.lpszClassName = L"TestWClass";
     RegisterClass(&wc);
 
     hwnd =
-        CreateWindowEx(0, "TestWClass", "TestWClass", WS_OVERLAPPEDWINDOW,
+        CreateWindowEx(0, L"TestWClass", L"TestWClass", WS_OVERLAPPEDWINDOW,
                        CW_USEDEFAULT, CW_USEDEFAULT, CW_USEDEFAULT,
                        CW_USEDEFAULT, (HWND) NULL, (HMENU) NULL,
                        GetModuleHandle(NULL), (LPVOID) NULL);
@@ -248,7 +248,7 @@ void initAsyncSigs(void (*sigcleanup)(int))
 	    }
         }
     }
-    HANDLE hInvisiblethread =
+
 	CreateThread(NULL, 0, RunInvisibleWindowThread, NULL, 0, &tid);
     SetConsoleCtrlHandler((PHANDLER_ROUTINE)CtrlHandler, TRUE);
     eWorkFinished = CreateEvent(NULL, TRUE, FALSE, NULL);

src/internfile/mimehandler.cpp

@@ -43,6 +43,7 @@ using namespace std;
 #include "mh_null.h"
 #include "mh_xslt.h"
 #include "rcldoc.h"
+#include "rclutil.h"
 
 // Performance help: we use a pool of already known and created
 // handlers. There can be several instances for a given mime type
@@ -135,6 +136,7 @@ void clearMimeHandlerCache()
 		delete it->second;
     }
     o_handlers.clear();
+    TempFile::tryRemoveAgain();
 }
 
 /** For mime types set as "internal" in mimeconf: 

src/testmains/trrclutil.cpp

@@ -0,0 +1,51 @@
+
+#include "rclutil.h"
+
+
+void path_to_thumb(const string& _input)
+{
+    string input(_input);
+    // Make absolute path if needed
+    if (input[0] != '/') {
+        input = path_absolute(input);
+    }
+
+    input = string("file://") + path_canon(input);
+
+    string path;
+    //path = url_encode(input, 7);
+    thumbPathForUrl(input, 7, path);
+    cout << path << endl;
+}
+
+const char *thisprog;
+
+int main(int argc, const char **argv)
+{
+    thisprog = *argv++;
+    argc--;
+
+    string s;
+    vector<string>::const_iterator it;
+
+#if 0
+    if (argc > 1) {
+        cerr <<  "Usage: thumbpath <filepath>" << endl;
+        exit(1);
+    }
+    string input;
+    if (argc == 1) {
+        input = *argv++;
+        if (input.empty())  {
+            cerr << "Usage: thumbpath <filepath>" << endl;
+            exit(1);
+        }
+        path_to_thumb(input);
+    } else {
+        while (getline(cin, input)) {
+            path_to_thumb(input);
+        }
+    }
+    exit(0);
+#endif
+}

src/utils/pathut.cpp

@@ -274,7 +274,7 @@ bool fsocc(const string& path, int *pc, long long *avmbs)
 #ifdef _WIN32
     ULARGE_INTEGER freebytesavail;
     ULARGE_INTEGER totalbytes;
-    if (!GetDiskFreeSpaceEx(path.c_str(), &freebytesavail,
+    if (!GetDiskFreeSpaceExA(path.c_str(), &freebytesavail,
 							 &totalbytes, NULL)) {
         return false;
     }

src/utils/rclutil.cpp

@@ -1,4 +1,4 @@
-/* Copyright (C) 2016 J.F.Dockes
+/* Copyright (C) 2016-2019 J.F.Dockes
  *   This program is free software; you can redistribute it and/or modify
  *   it under the terms of the GNU General Public License as published by
  *   the Free Software Foundation; either version 2 of the License, or
@@ -14,7 +14,7 @@
  *   Free Software Foundation, Inc.,
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  */
-#ifndef TEST_RCLUTIL
+
 #include "autoconfig.h"
 
 #include <stdio.h>
@@ -39,6 +39,7 @@
 #include <mutex>
 #include <map>
 #include <unordered_map>
+#include <list>
 
 #include "rclutil.h"
 #include "pathut.h"
@@ -50,7 +51,6 @@
 
 using namespace std;
 
-
 template <class T> void map_ss_cp_noshr(T s, T *d)
 {
     for (const auto& ent : s) {
@@ -83,54 +83,48 @@ static bool path_isdriveabs(const string& s)
 #include <Shlwapi.h>
 #pragma comment(lib, "shlwapi.lib")
 
-string path_tchartoutf8(TCHAR *text)
-{
-#ifdef UNICODE
-    // Simple C
-    // const size_t size = ( wcslen(text) + 1 ) * sizeof(wchar_t);
-    // wcstombs(&buffer[0], text, size);
-    // std::vector<char> buffer(size);
-    // Or:
-    // Windows API
-    std::vector<char> buffer;
-    int size = WideCharToMultiByte(CP_UTF8, 0, text, -1, NULL, 0, NULL, NULL);
-    if (size > 0) {
-        buffer.resize(size);
-        WideCharToMultiByte(CP_UTF8, 0, text, -1,
-                            &buffer[0], int(buffer.size()), NULL, NULL);
-    } else {
-        return string();
-    }
-    return string(&buffer[0]);
-#else
-    return text;
-#endif
-}
 string path_thisexecpath()
 {
-    TCHAR text[MAX_PATH];
+    wchar_t text[MAX_PATH];
-    GetModuleFileName(NULL, text, MAX_PATH);
+    GetModuleFileNameW(NULL, text, MAX_PATH);
 #ifdef NTDDI_WIN8_future
     PathCchRemoveFileSpec(text, MAX_PATH);
 #else
-    PathRemoveFileSpec(text);
+    PathRemoveFileSpecW(text);
 #endif
-    string path = path_tchartoutf8(text);
+    string path;
+    wchartoutf8(text, path);
     if (path.empty()) {
         path = "c:/";
     }
 
     return path;
 }
-string path_wingettempfilename(TCHAR *pref)
+string path_wingettempfilename(wchar_t *pref)
 {
-    TCHAR buf[(MAX_PATH + 1)*sizeof(TCHAR)];
+    // Use a subdirectory named "rcltmp" inside the windows temp
-    TCHAR dbuf[(MAX_PATH + 1)*sizeof(TCHAR)];
+    // location.
-    GetTempPath(MAX_PATH + 1, dbuf);
+    wchar_t dbuf[MAX_PATH + 1];
-    GetTempFileName(dbuf, pref, 0, buf);
+    GetTempPathW(MAX_PATH + 1, dbuf);
+    string tdir;
+    wchartoutf8(dbuf, tdir);
+    tdir = path_cat(tdir, "rcltmp");;
+    if (!path_exists(tdir)) {
+        if (path_makepath(tdir, 0700)) {
+            LOGSYSERR("path_wingettempfilename", "path_makepath", tdir);
+        }
+    }
+    utf8towchar(tdir, dbuf, MAX_PATH);
+    wchar_t buf[MAX_PATH + 1];
+    GetTempFileNameW(dbuf, pref, 0, buf);
     // Windows will have created a temp file, we delete it.
-    string filename = path_tchartoutf8(buf);
+    if (!DeleteFileW(buf)) {
-    unlink(filename.c_str());
+        LOGSYSERR("path_wingettempfilename", "DeleteFileW", filename);
+    } else {
+        LOGDEB1("path_wingettempfilename: DeleteFile " << filename << " Ok\n");
+    }
+    string filename;
+    wchartoutf8(buf, filename);
     path_slashize(filename);
     return filename;
 }
@@ -235,9 +229,9 @@ const string& tmplocation()
         }
         if (tmpdir == 0) {
 #ifdef _WIN32
-            TCHAR bufw[(MAX_PATH + 1)*sizeof(TCHAR)];
+            wchar_t bufw[MAX_PATH + 1];
-            GetTempPath(MAX_PATH + 1, bufw);
+            GetTempPathW(MAX_PATH + 1, bufw);
-            stmpdir = path_tchartoutf8(bufw);
+            wchartoutf8(bufw, stmpdir);
 #else
             stmpdir = "/tmp";
 #endif
@@ -267,7 +261,7 @@ bool maketmpdir(string& tdir, string& reason)
     // in the foot
 #if !defined(HAVE_MKDTEMP) || defined(_WIN32)
     static std::mutex mmutex;
-    std::unique_lock lock(mmutex);
+    std::unique_lock<std::mutex> lock(mmutex);
 #endif
 
     if (!
@@ -291,7 +285,8 @@ bool maketmpdir(string& tdir, string& reason)
     // in the foot
     static std::mutex mmutex;
     std::unique_lock<std::mutex> lock(mmutex);
-    tdir = path_wingettempfilename(TEXT("rcltmp"));
+    static wchar_t tmpbasename[]{L"rcltmp"};
+    tdir = path_wingettempfilename(tmpbasename);
 #endif
 
     // At this point the directory does not exist yet except if we used
@@ -382,10 +377,12 @@ TempFile::Internal::Internal(const string& suffix)
     filename = cp;
     free(cp);
 #else
-    string filename = path_wingettempfilename(TEXT("recoll"));
+    static wchar_t tmpbasename[]{L"rcl"};
+    string filename = path_wingettempfilename(tmpbasename);
 #endif
 
     m_filename = filename + suffix;
+    LOGDEB1("TempFile: filename: " << m_filename << endl);
     int fd1 = open(m_filename.c_str(), O_CREAT | O_EXCL, 0600);
     if (fd1 < 0) {
         m_reason = string("Open/create error. errno : ") +
@@ -396,11 +393,49 @@ TempFile::Internal::Internal(const string& suffix)
     }
 }
 
+#ifdef _WIN32
+static list<string> remainingTempFileNames;
+static std::mutex remTmpFNMutex;
+#endif
+
 TempFile::Internal::~Internal()
 {
     if (!m_filename.empty() && !m_noremove) {
-        unlink(m_filename.c_str());
+        LOGDEB1("TempFile:~: unlinking " << m_filename << endl);
+        if (unlink(m_filename.c_str()) != 0) {
+            LOGSYSERR("TempFile:~", "unlink", m_filename);
+#ifdef _WIN32
+            {
+                std::unique_lock<std::mutex> lock(remTmpFNMutex);
+                remainingTempFileNames.push_back(m_filename);
             }
+#endif
+        } else {
+            LOGDEB1("TempFile:~: unlink " << m_filename << " Ok\n");
+        }
+    }
+}
+
+// On Windows we sometimes fail to remove temporary files because
+// they are open. It's difficult to make sure this does not
+// happen, so we add a cleaning pass after clearing the input
+// handlers cache (which should kill subprocesses etc.)
+void TempFile::tryRemoveAgain()
+{
+#ifdef _WIN32
+	LOGDEB1("TempFile::tryRemoveAgain. List size: " <<
+			remainingTempFileNames.size() << endl);
+    std::unique_lock<std::mutex> lock(remTmpFNMutex);
+    std::list<string>::iterator pos = remainingTempFileNames.begin();
+    while (pos != remainingTempFileNames.end()) {
+        if (unlink(pos->c_str()) != 0) {
+            LOGSYSERR("TempFile::tryRemoveAgain", "unlink", *pos);
+            pos++;
+        } else {
+            pos = remainingTempFileNames.erase(pos);
+        }
+    }
+#endif
 }
 
 TempDir::TempDir()
@@ -448,6 +483,7 @@ static const string& xdgcachedir()
     }
     return xdgcache;
 }
+
 static const string& thumbnailsdir()
 {
     static string thumbnailsd;
@@ -506,56 +542,3 @@ void rclutil_init_mt()
     tmplocation();
     thumbnailsdir();
 }
-
-#else // TEST_RCLUTIL
-
-void path_to_thumb(const string& _input)
-{
-    string input(_input);
-    // Make absolute path if needed
-    if (input[0] != '/') {
-        input = path_absolute(input);
-    }
-
-    input = string("file://") + path_canon(input);
-
-    string path;
-    //path = url_encode(input, 7);
-    thumbPathForUrl(input, 7, path);
-    cout << path << endl;
-}
-
-const char *thisprog;
-
-int main(int argc, const char **argv)
-{
-    thisprog = *argv++;
-    argc--;
-
-    string s;
-    vector<string>::const_iterator it;
-
-#if 0
-    if (argc > 1) {
-        cerr <<  "Usage: thumbpath <filepath>" << endl;
-        exit(1);
-    }
-    string input;
-    if (argc == 1) {
-        input = *argv++;
-        if (input.empty())  {
-            cerr << "Usage: thumbpath <filepath>" << endl;
-            exit(1);
-        }
-        path_to_thumb(input);
-    } else {
-        while (getline(cin, input)) {
-            path_to_thumb(input);
-        }
-    }
-    exit(0);
-#endif
-}
-
-#endif // TEST_RCLUTIL
-

src/utils/rclutil.h

@@ -58,6 +58,11 @@ public:
     const std::string& getreason() const;
     void setnoremove(bool onoff);
     bool ok() const;
+	// Attempt to delete all files which could not be deleted on the
+	// first try (typically on Windows: because they are open by some
+	// process). Called after clearing the mimeHandler cache. Does
+	// nothing if not _WIN32
+	static void tryRemoveAgain();
     class Internal;
 private:
     std::shared_ptr<Internal> m;

src/windows/execmd_w.cpp

@@ -108,6 +108,13 @@ static string argvToCmdLine(const string& cmd, const vector<string>& args)
     return cmdline;
 }
 
+// Because we build with UNICODE defined, GetEnvironmentStrings is
+// defined as GetEnvironmentStringsW. Because of a Windows problem teh
+// GetEnvironmentStrings function is really a GetEnvironmentStringsA,
+// which is what we want.
+// See: https://devblogs.microsoft.com/oldnewthing/20130117-00/?p=5533
+#undef GetEnvironmentStrings
+
 // Merge the father environment with the variable specified in m_env
 static char *mergeEnvironment(const std::unordered_map<string, string>& addenv)
 {
@@ -131,7 +138,7 @@ static char *mergeEnvironment(const std::unordered_map<string, string>& addenv)
         }
     }
 
-    FreeEnvironmentStrings(envir);
+    FreeEnvironmentStringsA(envir);
 
     // Merge our values
     for (auto it = addenv.begin(); it != addenv.end(); it++) {
@@ -614,7 +621,7 @@ bool ExecCmd::Internal::preparePipes(bool has_input,HANDLE *hChildInput,
         // attribute, and the options are important.
         // use CreateFile to open a new handle to the existing pipe...
         sa.bInheritHandle = TRUE; 
-        hOutputWrite = CreateFile(
+        hOutputWrite = CreateFileA(
             pipeName.c_str(),
             FILE_WRITE_DATA | SYNCHRONIZE,
             0, &sa, OPEN_EXISTING, // very important flag!
@@ -659,7 +666,7 @@ bool ExecCmd::Internal::preparePipes(bool has_input,HANDLE *hChildInput,
         }
 
         sa.bInheritHandle = TRUE;
-        hInputRead = CreateFile(
+        hInputRead = CreateFileA(
             pipeName.c_str(),
             FILE_READ_DATA | SYNCHRONIZE,
             0, &sa, OPEN_EXISTING, // very important flag!
@@ -782,13 +789,7 @@ int ExecCmd::startExec(const string &cmd, const vector<string>& args,
     char *envir = mergeEnvironment(m->m_env);
 
     // Create the child process. 
-    // Need a writable buffer for the command line, for some reason.
     LOGDEB("ExecCmd:startExec: cmdline [" << cmdline << "]\n");
-#if 0
-    LPSTR buf = (LPSTR)malloc(cmdline.size() + 1);
-    memcpy(buf, cmdline.c_str(), cmdline.size());
-    buf[cmdline.size()] = 0;
-#endif
     SYSPATH(cmdline, wcmdline);
     bSuccess = CreateProcessW(NULL,
                              wcmdline, // command line 
@@ -805,7 +806,6 @@ int ExecCmd::startExec(const string &cmd, const vector<string>& args,
     }
     
     free(envir);
-//    free(buf);
     // Close child-side handles else we'll never see eofs
     if (!CloseHandle(hOutputWrite))
         printError("CloseHandle");

src/windows/qmkrecoll/librecoll.pro

@@ -10,9 +10,7 @@ TARGET = librecoll
 TEMPLATE = lib
 
 DEFINES += LIBRECOLL_LIBRARY BUILDING_RECOLL
-DEFINES -= UNICODE
+DEFINES += UNICODE
-DEFINES -= _UNICODE
-DEFINES += _MBCS
 DEFINES += PSAPI_VERSION=1
 DEFINES += READFILE_ENABLE_MINIZ
 DEFINES += READFILE_ENABLE_MD5

src/windows/qmkrecoll/rclstartw.pro

@@ -5,9 +5,7 @@ TARGET = rclstartw
 TEMPLATE = app
 
 DEFINES += BUILDING_RECOLL
-DEFINES -= UNICODE
+DEFINES += UNICODE
-DEFINES -= _UNICODE
-DEFINES += _MBCS
 DEFINES += PSAPI_VERSION=1
 
 

src/windows/qmkrecoll/recollindex.pro

@@ -7,9 +7,7 @@ CONFIG -= app_bundle
 TEMPLATE = app
 
 DEFINES += BUILDING_RECOLL
-DEFINES -= UNICODE
+DEFINES += UNICODE
-DEFINES -= _UNICODE
-DEFINES += _MBCS
 DEFINES += PSAPI_VERSION=1
 DEFINES += RCL_MONITOR
 

src/windows/qmkrecoll/recollq.pro

@@ -7,9 +7,7 @@ CONFIG -= app_bundle
 TEMPLATE = app
 
 DEFINES += BUILDING_RECOLL
-DEFINES -= UNICODE
+DEFINES += UNICODE
-DEFINES -= _UNICODE
-DEFINES += _MBCS
 DEFINES += PSAPI_VERSION=1