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

[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



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20100403/cfce778f/attachment.htm>