[Mono-dev] CoreCLR patches

[Rodrigo Kumpera]

Hi Sebastien,

I have some comments on parts of your mono changes:


@@ -16,6 +16,7 @@
 #include "security-core-clr.h"

 gboolean mono_security_core_clr_test = FALSE;
+char* mono_security_core_clr_prefix = NULL;*
*
Why this field is not static?
Why do we even need it? mono_security_enable_core_clr stores on it and
nobody reads from.

+
+static char* platform_code_assemblies [] = {
+    "mscorlib",

This should be const.


+/*
+ * mono_security_core_clr_determine_platform_image:
+ *
+ *   Check if this image represent platform code
+ */
+gboolean
+mono_security_core_clr_determine_platform_image (MonoImage *image)
+{
+    /* FIXME: existing code not safe enough wrt XAP XXX we need to better
define *platform* code (e.g. location)

This part of the patch comments out a chunk of code that just doesn't fit
moon, right?
Then either remove the dead code or use explicit lines to add comment
start/end, otherwise it gets pretty tricky to figure it out.

@@ -203,7 +203,7 @@
 mono_get_exception_class    (void);

 void
-mono_security_enable_core_clr (void);
+mono_security_enable_core_clr (const char *prefix);

This breaks our ABI, we can't change that function this way.
I know it's a silly requirement for a function that hardly anyone could be
using, but
breaking our ABI should not be taken lightly.
Create a new function and leave a comment stating that
mono_security_enable_core_clr is deprecated
and will be eventually removed. This is bound to happen once we do a release
with sgen-gc enabled.

@@ -4481,28 +4507,60 @@
     mono_emit_method_call (cfg, thrower, NULL, NULL);
 }

+static MonoMethod*
+unwrap (MonoMethod *method)
+{


Please use a more meaningful name for this function.

 static gboolean
 method_is_safe (MonoMethod *method)
 {
-    /*
+    /* FIXME: looks somewhat incomplete

I think this is just dead code used during the initial development of
core-clr security.



2009/3/13 Sebastien Pouliot <sebastien.pouliot at gmail.com>

> Hello,
>
> With the set of attached patches all existing moon-unit tests pass. It
> also pass the SecurityCriticalTest[1] - which calls from application
> code (i.e. transparent) every visible SL2 API decorated as Critical.
>
> The runtime patch mainly avoid unneeded, repetitive calls so it should
> help performance - however I made those changes so it was easier to set
> breakpoints using gdb/xdb, real optimizations are for another day ;-)
>
> Next steps are to fix CoreCLR-related ReflectionTest[1] and
> ReflectionEmitTest[1] unit tests and, of course, more tests/testing.
>
> Thanks
> Sebastien
>
> [1] in SVN but ignored at the moment
>
>
> _______________________________________________
> Mono-devel-list mailing list
> Mono-devel-list at lists.ximian.com
> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.ximian.com/pipermail/mono-devel-list/attachments/20090313/89a2ed88/attachment.html