On Thu, Sep 3, 2009 at 12:36 PM, Paolo Molaro <lu...@ximian.com> wrote:
> I'm not happy with both the MONO_ERROR_STORE_FULL_MESSAGE and
> MONO_ERROR_DONT_COPY_STRINGS flags you introduced.
> For the message this could be handled automatically: if it doesn't fit
> into the buffer you can malloc it at set time, the code that initializes
> the
> MonoError has no way to know if the message will fit or not.
> Similarly, the initializing code can't know if any of the strings need
> to be strduped because it's far away from the code that sets it.
> This should be decided at the callsite, either passing a flag or calling
> a separate function that does the dup.
>
I introduced mono_error_dup_strings which let's the caller control if the
supplied
strings are to be released.
> A small note on overlong messages. I think we should have a policy like
> this:
> *) error messages should be short sentences
> *) any non-essential description of the error and suggestions for
> workarounds should go into either or both the logging mechanism and the
> documentation (wiki etc.)
>
Makes sense, one thing we could improve in the logged messages is to provide
some sort of unique code describing it. This helps a lot troubleshooting
users
problems and is very google friendly.
> > But what should be done when we get an allocation failure on the
> > mono_exception_set_*
> > function? Should we just convert such error into an OOM, abort due to a
> > double-fault or
> > silently ignore those arguments (and print something on console).
>
> It is tricky. In some cases we want to communicate an OOM exception to
> the managed code. In some others we can't: we need to give the caller
> code a chance to handle the error and not crash.
> Using a flag here can be helpful: something like MONO_ERROR_INCOMPLETE
> to signal that the full info was not stored for whatever reason.
> That should about strike a good balance between being robust and keeping
> track of as much info as we can. Aborting here is not an option, IMHO.
>
I added a MONO_ERROR_INCOMPLETE used for that.
> +void
> > +mono_error_set_corlib_exception (MonoError *oerror, const char
> *name_space, const char *name)
> > +{
> > + MonoErrorInternal *error = (MonoErrorInternal*)oerror;
> > +
> > + if (error->flags & MONO_ERROR_DONT_COPY_STRINGS) {
> > + error->exception_name_space = name_space;
> > + error->exception_name = name;
> > + } else {
> > + error->exception_name_space = g_strdup (name_space);
> > + error->exception_name = g_strdup (name);
> > + }
>
> This should set error_code as well (maybe have it passed as an argument).
>
>
I removed those functions from the API as they add no value at all. The way
to go is to
use one of the provided error-setting functions. Using those setter
functions won't make
difference for reporting the error.
If it's not enough, then it's better to just add another entry in the error
codes array and a
new error-setting function.
Attached is a revised patch.
Cheers,
Rodrigo
diff --git a/mono/utils/Makefile.am b/mono/utils/Makefile.am
index 4af317d..463f98a 100644
--- a/mono/utils/Makefile.am
+++ b/mono/utils/Makefile.am
@@ -61,7 +61,10 @@ libmonoutils_la_SOURCES = \
freebsd-elf64.h \
freebsd-dwarf.h \
dtrace.h \
- gc_wrapper.h
+ gc_wrapper.h \
+ mono-error.c \
+ mono-error.h \
+ mono-error-internal.h
libmonoutilsincludedir = $(includedir)/mono-$(API_VER)/mono/utils
diff --git a/mono/utils/mono-error-internals.h b/mono/utils/mono-error-internals.h
new file mode 100644
index 0000000..b7e6137
--- /dev/null
+++ b/mono/utils/mono-error-internals.h
@@ -0,0 +1,58 @@
+#ifndef __MONO_ERROR_INTERNALS_H__
+#define __MONO_ERROR_INTERNALS_H__
+
+#include "mono/utils/mono-compiler.h"
+#include "mono/metadata/object-internals.h"
+
+/*Keep in sync with MonoError*/
+typedef struct {
+ unsigned short error_code;
+ unsigned short flags;
+
+ const char *type_name;
+ const char *assembly_name;
+ const char *member_name;
+ const char *exception_name_space;
+ const char *exception_name;
+ MonoClass *klass;
+ const char *full_message;
+
+ void *padding [5];
+ char message [128];
+} MonoErrorInternal;
+
+void
+mono_error_dup_strings (MonoError *error, gboolean dup_strings) MONO_INTERNAL;
+
+/* This function is not very useful as you can't provide any details beyond the message.*/
+void
+mono_error_set_error (MonoError *error, int error_code, const char *msg_format, ...) MONO_INTERNAL;
+
+void
+mono_error_set_assembly_load (MonoError *error, const char *assembly_name, const char *msg_format, ...) MONO_INTERNAL;
+
+void
+mono_error_set_type_load_class (MonoError *error, MonoClass *klass, const char *msg_format, ...) MONO_INTERNAL;
+
+void
+mono_error_set_type_load_name (MonoError *error, const char *type_name, const char *assembly_name, const char *msg_format, ...) MONO_INTERNAL;
+
+void
+mono_error_set_method_load (MonoError *error, MonoClass *klass, const char *method_name, const char *msg_format, ...) MONO_INTERNAL;
+
+void
+mono_error_set_field_load (MonoError *error, MonoClass *klass, const char *field_name, const char *msg_format, ...) MONO_INTERNAL;
+
+void
+mono_error_set_bad_image (MonoError *error, const char *file_name, const char *msg_format, ...) MONO_INTERNAL;
+
+void
+mono_error_set_out_of_memory (MonoError *error, const char *msg_format, ...) MONO_INTERNAL;
+
+void
+mono_error_set_generic_error (MonoError *error, const char * name_space, const char *name, const char *msg_format, ...) MONO_INTERNAL;
+
+MonoException*
+mono_error_prepare_exception (MonoError *error, MonoError *error_out) MONO_INTERNAL;
+
+#endif
diff --git a/mono/utils/mono-error.c b/mono/utils/mono-error.c
new file mode 100644
index 0000000..6ccd7cb
--- /dev/null
+++ b/mono/utils/mono-error.c
@@ -0,0 +1,414 @@
+/*
+ * mono-error.c: Error handling code
+ *
+ * Authors:
+ * Rodrigo Kumpera (rkump...@novell.com)
+ * Copyright 2009 Novell, Inc (http://www.novell.com)
+ */
+#include <glib.h>
+
+#include "mono-error.h"
+#include "mono-error-internals.h"
+
+#include <mono/metadata/exception.h>
+#include <mono/metadata/object-internals.h>
+#include <mono/metadata/debug-helpers.h>
+
+#define mono_error_get_message(E) ((E)->full_message ? (E)->full_message : (E)->message)
+
+#define set_error_message() do { \
+ va_list args; \
+ va_start (args, msg_format); \
+ if (g_vsnprintf (error->message, sizeof (error->message), msg_format, args) >= sizeof (error->message)) {\
+ if (!(error->full_message = g_strdup_vprintf (msg_format, args))) \
+ error->flags |= MONO_ERROR_INCOMPLETE; \
+ } \
+ va_end (args); \
+} while (0)
+
+static void
+mono_error_prepare (MonoErrorInternal *error)
+{
+ if (error->error_code != MONO_ERROR_NONE)
+ return;
+
+ error->type_name = error->assembly_name = error->member_name = error->full_message = error->exception_name_space = error->exception_name = NULL;
+ error->klass = NULL;
+ error->message [0] = 0;
+}
+
+void
+mono_error_init_flags (MonoError *oerror, unsigned short flags)
+{
+ MonoErrorInternal *error = (MonoErrorInternal*)oerror;
+ g_assert (sizeof (MonoError) == sizeof (MonoErrorInternal));
+
+ error->error_code = MONO_ERROR_NONE;
+ error->flags = flags;
+}
+
+void
+mono_error_init (MonoError *error)
+{
+ mono_error_init_flags (error, 0);
+}
+
+void
+mono_error_cleanup (MonoError *oerror)
+{
+ MonoErrorInternal *error = (MonoErrorInternal*)oerror;
+ if (error->error_code == MONO_ERROR_NONE)
+ return;
+
+ g_free ((char*)error->full_message);
+ if (!(error->flags & MONO_ERROR_FREE_STRINGS)) //no memory was allocated
+ return;
+
+ g_free ((char*)error->type_name);
+ g_free ((char*)error->assembly_name);
+ g_free ((char*)error->member_name);
+ g_free ((char*)error->exception_name_space);
+ g_free ((char*)error->exception_name);
+}
+
+gboolean
+mono_error_ok (MonoError *error)
+{
+ return error->error_code == MONO_ERROR_NONE;
+}
+
+unsigned short
+mono_error_get_error_code (MonoError *error)
+{
+ return error->error_code;
+}
+
+/*
+ * Inform that this error has heap allocated strings.
+ * The strings will be duplicated if @dup_strings is TRUE
+ * otherwise they will just be free'd in mono_error_cleanup.
+ */
+void
+mono_error_dup_strings (MonoError *oerror, gboolean dup_strings)
+{
+#define DUP_STR(field) do { if (error->field) {\
+ if (!(error->field = g_strdup (error->field))) \
+ error->flags |= MONO_ERROR_INCOMPLETE; \
+ }} while (0);
+
+ MonoErrorInternal *error = (MonoErrorInternal*)oerror;
+ error->flags |= MONO_ERROR_FREE_STRINGS;
+
+ if (dup_strings) {
+ DUP_STR (type_name);
+ DUP_STR (assembly_name);
+ DUP_STR (member_name);
+ DUP_STR (exception_name_space);
+ DUP_STR (exception_name);
+ }
+#undef DUP_STR
+}
+
+void
+mono_error_set_error (MonoError *oerror, int error_code, const char *msg_format, ...)
+{
+ MonoErrorInternal *error = (MonoErrorInternal*)oerror;
+ mono_error_prepare (error);
+
+ error->error_code = error_code;
+ set_error_message ();
+}
+
+static void
+mono_error_set_assembly_name (MonoError *oerror, const char *assembly_name)
+{
+ MonoErrorInternal *error = (MonoErrorInternal*)oerror;
+ g_assert (error->error_code != MONO_ERROR_NONE);
+
+ error->assembly_name = assembly_name;
+}
+
+static void
+mono_error_set_member_name (MonoError *oerror, const char *member_name)
+{
+ MonoErrorInternal *error = (MonoErrorInternal*)oerror;
+
+ error->member_name = member_name;
+}
+
+static void
+mono_error_set_type_name (MonoError *oerror, const char *type_name)
+{
+ MonoErrorInternal *error = (MonoErrorInternal*)oerror;
+
+ error->type_name = type_name;
+}
+
+static void
+mono_error_set_class (MonoError *oerror, MonoClass *klass)
+{
+ MonoErrorInternal *error = (MonoErrorInternal*)oerror;
+
+ error->klass = klass;
+}
+
+static void
+mono_error_set_corlib_exception (MonoError *oerror, const char *name_space, const char *name)
+{
+ MonoErrorInternal *error = (MonoErrorInternal*)oerror;
+
+ error->exception_name_space = name_space;
+ error->exception_name = name;
+}
+
+
+void
+mono_error_set_assembly_load (MonoError *oerror, const char *assembly_name, const char *msg_format, ...)
+{
+ MonoErrorInternal *error = (MonoErrorInternal*)oerror;
+ mono_error_prepare (error);
+
+ error->error_code = MONO_ERROR_FILE_NOT_FOUND;
+ mono_error_set_assembly_name (oerror, assembly_name);
+
+ set_error_message ();
+}
+
+void
+mono_error_set_type_load_class (MonoError *oerror, MonoClass *klass, const char *msg_format, ...)
+{
+ MonoErrorInternal *error = (MonoErrorInternal*)oerror;
+ mono_error_prepare (error);
+
+ error->error_code = MONO_ERROR_TYPE_LOAD;
+ mono_error_set_class (oerror, klass);
+ set_error_message ();
+}
+
+void
+mono_error_set_type_load_name (MonoError *oerror, const char *type_name, const char *assembly_name, const char *msg_format, ...)
+{
+ MonoErrorInternal *error = (MonoErrorInternal*)oerror;
+ mono_error_prepare (error);
+
+ error->error_code = MONO_ERROR_TYPE_LOAD;
+ mono_error_set_type_name (oerror, type_name);
+ mono_error_set_assembly_name (oerror, assembly_name);
+ set_error_message ();
+}
+
+void
+mono_error_set_method_load (MonoError *oerror, MonoClass *klass, const char *method_name, const char *msg_format, ...)
+{
+ MonoErrorInternal *error = (MonoErrorInternal*)oerror;
+ mono_error_prepare (error);
+
+ error->error_code = MONO_ERROR_MISSING_METHOD;
+ mono_error_set_class (oerror, klass);
+ mono_error_set_member_name (oerror, method_name);
+ set_error_message ();
+}
+
+void
+mono_error_set_field_load (MonoError *oerror, MonoClass *klass, const char *field_name, const char *msg_format, ...)
+{
+ MonoErrorInternal *error = (MonoErrorInternal*)oerror;
+ mono_error_prepare (error);
+
+ error->error_code = MONO_ERROR_MISSING_FIELD;
+ mono_error_set_class (oerror, klass);
+ mono_error_set_member_name (oerror, field_name);
+ set_error_message ();
+}
+
+void
+mono_error_set_bad_image (MonoError *oerror, const char *assembly_name, const char *msg_format, ...)
+{
+ MonoErrorInternal *error = (MonoErrorInternal*)oerror;
+ mono_error_prepare (error);
+
+ error->error_code = MONO_ERROR_BAD_IMAGE;
+ mono_error_set_assembly_name (oerror, assembly_name);
+ set_error_message ();
+}
+
+void
+mono_error_set_generic_error (MonoError *oerror, const char * name_space, const char *name, const char *msg_format, ...)
+{
+ MonoErrorInternal *error = (MonoErrorInternal*)oerror;
+ mono_error_prepare (error);
+
+ error->error_code = MONO_ERROR_GENERIC;
+ mono_error_set_corlib_exception (oerror, name_space, name);
+ set_error_message ();
+}
+
+void
+mono_error_set_out_of_memory (MonoError *oerror, const char *msg_format, ...)
+{
+ MonoErrorInternal *error = (MonoErrorInternal*)oerror;
+ va_list args;
+ mono_error_prepare (error);
+
+ error->error_code = MONO_ERROR_OUT_OF_MEMORY;
+ va_start (args, msg_format);
+ g_vsnprintf (error->message, sizeof (error->message), msg_format, args);
+ va_end (args);
+}
+
+static MonoString*
+get_type_name_as_mono_string (MonoErrorInternal *error, MonoDomain *domain, MonoError *error_out)
+{
+ MonoString* res = NULL;
+
+ if (error->type_name) {
+ res = mono_string_new (domain, error->type_name);
+
+ } else if (error->klass) {
+ char *name = mono_type_full_name (&error->klass->byval_arg);
+ if (name) {
+ res = mono_string_new (domain, name);
+ g_free (name);
+ }
+ }
+ if (!res)
+ mono_error_set_out_of_memory (error_out, "Could not allocate type name");
+ return res;
+}
+
+static void
+set_message_on_exception (MonoException *exception, MonoErrorInternal *error, MonoError *error_out)
+{
+ MonoString *msg = mono_string_new (mono_domain_get (), mono_error_get_message (error));
+ if (msg)
+ MONO_OBJECT_SETREF (exception, message, msg);
+ else
+ mono_error_set_out_of_memory (error_out, "Could not allocate exception object");
+}
+
+MonoException*
+mono_error_prepare_exception (MonoError *oerror, MonoError *error_out)
+{
+ MonoErrorInternal *error = (MonoErrorInternal*)oerror;
+
+ MonoException* exception = NULL;
+ MonoString *assembly_name = NULL, *type_name = NULL, *method_name = NULL, *field_name = NULL, *msg = NULL;
+ MonoDomain *domain = mono_domain_get ();
+
+
+ switch (error->error_code) {
+ case MONO_ERROR_NONE:
+ return NULL;
+
+ case MONO_ERROR_MISSING_METHOD:
+ if ((error->type_name || error->klass) && error->member_name) {
+ type_name = get_type_name_as_mono_string (error, domain, error_out);
+ if (!mono_error_ok (error_out))
+ break;
+
+ method_name = mono_string_new (domain, error->member_name);
+ if (!method_name) {
+ mono_error_set_out_of_memory (error_out, "Could not allocate method name");
+ break;
+ }
+
+ exception = mono_exception_from_name_two_strings (mono_defaults.corlib, "System", "MissingMethodException", type_name, method_name);
+ if (exception)
+ set_message_on_exception (exception, error, error_out);
+ } else {
+ exception = mono_exception_from_name_msg (mono_defaults.corlib, "System", "MissingMethodException", mono_error_get_message (error));
+ }
+ break;
+
+ case MONO_ERROR_MISSING_FIELD:
+ if ((error->type_name || error->klass) && error->member_name) {
+ type_name = get_type_name_as_mono_string (error, domain, error_out);
+ if (!mono_error_ok (error_out))
+ break;
+
+ field_name = mono_string_new (domain, error->member_name);
+ if (!field_name) {
+ mono_error_set_out_of_memory (error_out, "Could not allocate field name");
+ break;
+ }
+
+ exception = mono_exception_from_name_two_strings (mono_defaults.corlib, "System", "MissingFieldException", type_name, field_name);
+ if (exception)
+ set_message_on_exception (exception, error, error_out);
+ } else {
+ exception = mono_exception_from_name_msg (mono_defaults.corlib, "System", "MissingFieldException", mono_error_get_message (error));
+ }
+ break;
+
+ case MONO_ERROR_TYPE_LOAD:
+ if (error->type_name || error->assembly_name) {
+ type_name = get_type_name_as_mono_string (error, domain, error_out);
+ if (!mono_error_ok (error_out))
+ break;
+
+ if (error->assembly_name) {
+ assembly_name = mono_string_new (domain, error->assembly_name);
+ if (!assembly_name) {
+ mono_error_set_out_of_memory (error_out, "Could not allocate assembly name");
+ break;
+ }
+ }
+
+ exception = mono_exception_from_name_two_strings (mono_get_corlib (), "System", "TypeLoadException", type_name, assembly_name);
+ if (exception)
+ set_message_on_exception (exception, error, error_out);
+ } else {
+ exception = mono_exception_from_name_msg (mono_defaults.corlib, "System", "TypeLoadException", mono_error_get_message (error));
+ }
+ break;
+
+ case MONO_ERROR_FILE_NOT_FOUND:
+ case MONO_ERROR_BAD_IMAGE:
+ if (error->assembly_name) {
+ msg = mono_string_new (domain, mono_error_get_message (error));
+ if (!msg) {
+ mono_error_set_out_of_memory (error_out, "Could not allocate message");
+ break;
+ }
+
+ if (error->assembly_name) {
+ assembly_name = mono_string_new (domain, error->assembly_name);
+ if (!assembly_name) {
+ mono_error_set_out_of_memory (error_out, "Could not allocate assembly name");
+ break;
+ }
+ }
+
+ if (error->error_code == MONO_ERROR_FILE_NOT_FOUND)
+ exception = mono_exception_from_name_two_strings (mono_get_corlib (), "System.IO", "FileNotFoundException", msg, assembly_name);
+ else
+ exception = mono_exception_from_name_two_strings (mono_defaults.corlib, "System", "BadImageFormatException", msg, assembly_name);
+ } else {
+ if (error->error_code == MONO_ERROR_FILE_NOT_FOUND)
+ exception = mono_exception_from_name_msg (mono_get_corlib (), "System.IO", "FileNotFoundException", mono_error_get_message (error));
+ else
+ exception = mono_exception_from_name_msg (mono_defaults.corlib, "System", "BadImageFormatException", mono_error_get_message (error));
+ }
+ break;
+
+ case MONO_ERROR_OUT_OF_MEMORY:
+ exception = mono_get_exception_out_of_memory ();
+ break;
+
+ case MONO_ERROR_GENERIC:
+ if (!error->exception_name_space || !error->exception_name)
+ mono_error_set_generic_error (error_out, "System", "ExecutionEngineException", "MonoError with generic error but no exception name was supplied");
+ else
+ exception = mono_exception_from_name_msg (mono_defaults.corlib, error->exception_name_space, error->exception_name, mono_error_get_message (error));
+ break;
+
+ default:
+ mono_error_set_generic_error (error_out, "System", "ExecutionEngineException", "Invalid error-code %d", error->error_code);
+ }
+
+ if (!mono_error_ok (error_out))
+ return NULL;
+ if (!exception)
+ mono_error_set_out_of_memory (error_out, "Could not allocate exception object");
+ return exception;
+}
diff --git a/mono/utils/mono-error.h b/mono/utils/mono-error.h
new file mode 100644
index 0000000..cbd4526
--- /dev/null
+++ b/mono/utils/mono-error.h
@@ -0,0 +1,58 @@
+#ifndef __MONO_ERROR_H__
+#define __MONO_ERROR_H__
+
+#include <mono/metadata/class.h>
+#include <mono/metadata/metadata.h>
+
+enum {
+ /*
+ The supplied strings were dup'd by means of calling mono_error_dup_strings.
+ */
+ MONO_ERROR_FREE_STRINGS = 0x0001,
+
+ /*
+ Something happened while processing the error and the resulting message is incomplete.
+ */
+ MONO_ERROR_INCOMPLETE = 0x0002
+};
+
+enum {
+ MONO_ERROR_NONE = 0,
+ MONO_ERROR_MISSING_METHOD = 1,
+ MONO_ERROR_MISSING_FIELD = 2,
+ MONO_ERROR_TYPE_LOAD = 3,
+ MONO_ERROR_FILE_NOT_FOUND = 4,
+ MONO_ERROR_BAD_IMAGE = 5,
+ MONO_ERROR_OUT_OF_MEMORY = 6,
+ /*
+ * This is a generic error mechanism is you need to raise an arbitrary corlib exception.
+ * You must pass the exception name otherwise prepare_exception will fail with internal execution.
+ */
+ MONO_ERROR_GENERIC = 7
+};
+
+/*Keep in sync with MonoErrorInternal*/
+typedef struct {
+ unsigned short error_code;
+ unsigned short hidden_0; /*DON'T TOUCH */
+
+ void *hidden_1 [12]; /*DON'T TOUCH */
+ char hidden_2 [128]; /*DON'T TOUCH */
+} MonoError;
+
+void
+mono_error_init (MonoError *error);
+
+void
+mono_error_init_flags (MonoError *error, unsigned short flags);
+
+void
+mono_error_cleanup (MonoError *error);
+
+gboolean
+mono_error_ok (MonoError *error);
+
+unsigned short
+mono_error_get_error_code (MonoError *error);
+
+#endif
_______________________________________________
Mono-devel-list mailing list
Mono-devel-list@lists.ximian.com
http://lists.ximian.com/mailman/listinfo/mono-devel-list
