rfc: mscoree: Use the mono embedding api instead of invoking mono.exe.

[Vincent Povirk]

This is a patch that has mscoree.dll load mono.dll in-process rather
than starting mono.exe. This is needed so that (among other things),
the running process gets the correct values from GetCommandLine and
GetModuleFileName(0).

The code quality is probably not very good, so I'm sending it to
wine-devel for feedback first.

[Jacek Caban]

Hi Vincent,

On 4/2/10 11:16 PM, Vincent Povirk wrote:
> This is a patch that has mscoree.dll load mono.dll in-process rather
> than starting mono.exe. This is needed so that (among other things),
> the running process gets the correct values from GetCommandLine and
> GetModuleFileName(0).
>
> The code quality is probably not very good, so I'm sending it to
> wine-devel for feedback first.

I don't know almost anything about embedding mono, but I've reviewed 
your patch. It mostly looks good for me, so my comments are more 
suggestions than real problems.

+static LPWSTR get_mono_path(DWORD major_version, DWORD minor_version, LPCWSTR subpath)

AFAICS you don't use *_version arguments, so they should be removed.

+    size = len + sizeof(slash) + (lstrlenW(subpath) + 1) * sizeof(WCHAR);
      if (!(ret = HeapAlloc(GetProcessHeap(), 0, size))) return NULL;

      lstrcpyW(ret, root);
      lstrcatW(ret, slash);
-    lstrcatW(ret, mono_exe);
+    lstrcatW(ret, subpath);

It's not really important as it only causes the buffer to be one char too big, but you count nullbyte both in sizeof(slash) and strlen+1. (It's already existing code and a matter of style, but strcat for one char doesn't look nice).

+static LPWSTR get_mono_exe(void)
+{
+    static const WCHAR mono_exe[] = {'b','i','n','\\','m','o','n','o','.','e','x','e',' ',0};
+
+    return get_mono_path(0, 0, mono_exe);
+}

You don't use this function, so it should be removed.

+        mono_dll_path = get_mono_path(0, 0, mono_dll);
+        if (!mono_dll_path) goto end;
+
+        mono_bin_path = get_mono_path(0, 0, bin);
+        set_environment(mono_bin_path);
+
+        mono_lib_path = get_mono_path(0, 0, lib);
+        mono_etc_path = get_mono_path(0, 0, etc);

Each get_mono_path performs a few registry operations. It feels right to fetch the path once and simply concatenate subdirs later.

+        current_arg += WideCharToMultiByte(CP_UTF8, 0, argvw[i], -1, current_arg, size, NULL, NULL);

size is not the right size here, but it's not really important as there won't be an overflow anyway.

+    trace_size = GetEnvironmentVariableA("WINE_MONO_TRACE", trace_setting, sizeof(trace_setting)/sizeof(WCHAR));

      if (trace_size)
      {
-        lstrcatW(cmd_line, trace_switch_start);
-        lstrcatW(cmd_line, trace_setting);
-        lstrcatW(cmd_line, trace_switch_end);
+        mono_jit_set_trace_options(trace_setting);

This should probably go to separated patch.

Error handling in _CorExeMain could be improved.

+/* Mono 2.6 embedding */
+typedef struct _MonoDomain MonoDomain;
+typedef struct _MonoAssembly MonoAssembly;
+
+extern HMODULE mono_handle;
+
+extern void (*mono_config_parse)(const char *filename);
+extern MonoAssembly* (*mono_domain_assembly_open) (MonoDomain *domain, const char *name);
+extern void (*mono_jit_cleanup)(MonoDomain *domain);
+extern int (*mono_jit_exec)(MonoDomain *domain, MonoAssembly *assembly, int argc, char *argv[]);
+extern MonoDomain* (*mono_jit_init)(const char *file);
+extern int (*mono_jit_set_trace_options)(const char*  options);
+extern void (*mono_set_dirs)(const char *assembly_dir, const char *config_dir);

Most of these (all?) are not needed outside mscoree_main.c so they should be static.


Jacek