Re: gerror.c revisited
- From: Sebastian Wilhelmi <wilhelmi ira uka de>
- To: Gtk Development List <gtk-devel-list gnome org>
- Subject: Re: gerror.c revisited
- Date: Wed, 30 Aug 2000 17:03:00 +0200
[resent to the list, which I forgot the first time, only mailing Havoc]
Hi Havoc,
> I'm doing it this way in gexec.h:
> #define G_EXEC_ERROR g_exec_error_quark ()
>
> GQuark g_exec_error_quark();
>
> Then the convention for error codes is G_EXEC_ERROR_* members in
> GExecErrorType.
>
> I guess since you already can't use switch(), it may as well be a
> string. That makes sense to me. Anyone see a problem with it?
The attached patch implements this. I've g_strduped the domain. It should
however be possible to just assign it. But the copy-solution avoids stupid
surprises (for lets say unloaded modules or the like).
> > * I would very much like to see a function
> > g_print_error (GError **err, gboolean fatal), that prints out the error
> > in some canonical form and does abort, if fatal is true. Or two
> > functions, one for error, one for warning (without abort).
>
> Seems reasonable. On fatal, I would fprintf(stderr, message); then
> exit(1) rather than abort(), though. abort() means "should not
> happen", i.e. g_assert(), and GError should not be used to indicate
> programming bugs.
Ok, actually I don't need that function, if g_set_error is not supposed to
abort (or exit). That will also make Tim happy ;-)
> > * g_set_error should call g_print_error (the_error, TRUE), when err is
> > NULL. Because if I supply NULL as an err argument, I actually don't want
> > to know, in the calling code whether it fails, but I'm not calling out
> > for silent failure, when the error condition is occuring. Or let
> > g_set_error take a fatal argument as above.
> >
>
> No, NULL means you want to _ignore_ the error, because the error
> doesn't matter. If the error matters, you need to handle it. :-P If
> the error "should not happen", then don't return it as a GError, just
> print a g_warning().
Ok, I'm not completly convinced, but can't think of any good counter example
either. SO if it's policy, I wont debate it. Or maybe I will ;-) Thinking of
Exceptions as a rough equivalent. They just abort, when not handled, avoiding
silent failure. You say the .xsession-errors mess is bad. But what about a
program, that fails without writing something there. Ok. I'll shut up.
> > +#define glib_error_domain() g_quark_from_static_string ("glib-error-domain")
> > +#define GLIB_ERROR_AGAIN 1 /* Resource temporarily unavailable */
> > +
>
> I would do this as G_THREAD_ERROR (for the domain) and an enum
> GThreadErrorType with member G_THREAD_ERROR_AGAIN. The convention is
> debatable, but we should use the same one throughout glib.
Ok, done.
> > + G_THREAD_UF (thread_create, (g_thread_create_proxy, result,
> > + stack_size, joinable, bound, priority,
> > + &result->system_thread, err));
> > G_UNLOCK (g_thread_create);
> > + if (err && *err)
> > + {
> > + g_free (result);
> > + return NULL;
> > + }
> > return (GThread*) result;
> > }
>
> This code is probably wrong (I'm not sure, it depends on exactly what
> thread_create does). Basically the return value should not be affected
> by whether the user passsed in NULL for err. If you need to know
> whether an error occurred in _your_ code, then create your own error
> variable, check that it's non-NULL, then assign to the user's error if
> needed. I'm pretty sure gexec.c has an example, but something like:
Yes, but this code would work, if g_set_error would exit for NULL.
> GError *local_err = NULL;
> foo (&local_err);
> if (local_err != NULL)
> {
> if (err)
> *err = local_err;
> else
> g_error_free (local_err);
>
> return NULL;
> }
This last part looks like it will be used in a whole lot of functions and it
seems a bit tedious and easy to get wrong. So I included g_propagate_error for
that purpose, what do you think?
Bye,
Sebastian
--
Sebastian Wilhelmi | här ovanför alla molnen
mailto:wilhelmi@ira.uka.de | är himmlen så förunderligt blå
http://goethe.ira.uka.de/~wilhelmi |
Index: gerror.c
===================================================================
RCS file: /cvs/gnome/glib/gerror.c,v
retrieving revision 1.15
diff -u -b -B -r1.15 gerror.c
--- gerror.c 2000/07/26 11:01:59 1.15
+++ gerror.c 2000/08/30 14:19:53
@@ -27,7 +27,7 @@
#include "glib.h"
static GError*
-g_error_new_valist(GQuark domain,
+g_error_new_valist(const gchar *domain,
gint code,
const gchar *format,
va_list args)
@@ -36,7 +36,7 @@
error = g_new (GError, 1);
- error->domain = domain;
+ error->domain = g_strdup (domain);
error->code = code;
error->message = g_strdup_vprintf (format, args);
@@ -44,7 +44,7 @@
}
GError*
-g_error_new (GQuark domain,
+g_error_new (const gchar *domain,
gint code,
const gchar *format,
...)
@@ -63,7 +63,7 @@
}
GError*
-g_error_new_literal (GQuark domain,
+g_error_new_literal (const gchar *domain,
gint code,
const gchar *message)
{
@@ -74,7 +74,7 @@
err = g_new (GError, 1);
- err->domain = domain;
+ err->domain = g_strdup (domain);
err->code = code;
err->message = g_strdup (message);
@@ -88,6 +88,8 @@
g_free (error->message);
+ g_free (error->domain);
+
g_free (error);
}
@@ -109,17 +111,18 @@
gboolean
g_error_matches (const GError *error,
- GQuark domain,
+ const gchar *domain,
gint code)
{
return error &&
- error->domain == domain &&
- error->code == code;
+ error->code == code &&
+ error->domain && domain &&
+ strcmp (error->domain, domain) == 0;
}
void
g_set_error (GError **err,
- GQuark domain,
+ const gchar *domain,
gint code,
const gchar *format,
...)
@@ -136,6 +139,21 @@
va_start (args, format);
*err = g_error_new_valist (domain, code, format, args);
va_end (args);
+}
+
+void
+g_propagate_error (GError **dest,
+ GError *src)
+{
+ if (dest)
+ {
+ if (*dest != NULL)
+ g_warning ("GError set over the top of a previous GError or uninitialized memory.\n"
+ "This indicates a bug in someone's code. You must ensure an error is NULL before it's set.");
+ *dest = src;
+ }
+ else if (src)
+ g_error_free (src);
}
void
Index: gerror.h
===================================================================
RCS file: /cvs/gnome/glib/gerror.h,v
retrieving revision 1.2
diff -u -b -B -r1.2 gerror.h
--- gerror.h 2000/07/26 11:01:59 1.2
+++ gerror.h 2000/08/30 14:19:53
@@ -30,17 +30,17 @@
struct _GError
{
- GQuark domain;
+ gchar *domain;
gint code;
gchar *message;
};
-GError* g_error_new (GQuark domain,
+GError* g_error_new (const gchar *domain,
gint code,
const gchar *format,
...) G_GNUC_PRINTF (3, 4);
-GError* g_error_new_literal (GQuark domain,
+GError* g_error_new_literal (const gchar *domain,
gint code,
const gchar *message);
@@ -48,17 +48,23 @@
GError* g_error_copy (const GError *error);
gboolean g_error_matches (const GError *error,
- GQuark domain,
+ const gchar *domain,
gint code);
/* if (err) *err = g_error_new(domain, code, format, ...), also has
* some sanity checks.
*/
void g_set_error (GError **err,
- GQuark domain,
+ const gchar *domain,
gint code,
const gchar *format,
...) G_GNUC_PRINTF (4, 5);
+
+/* if (dest) *dest = src; else g_error_free (src) also has
+ * some sanity checks.
+ */
+void g_propagate_error (GError **dest,
+ GError *src);
/* if (err && *err) { g_error_free(*err); *err = NULL; } */
void g_clear_error (GError **err);
Index: glib.h
===================================================================
RCS file: /cvs/gnome/glib/glib.h,v
retrieving revision 1.190
diff -u -b -B -r1.190 glib.h
--- glib.h 2000/08/27 15:33:15 1.190
+++ glib.h 2000/08/30 14:19:54
@@ -941,7 +941,16 @@
guint len;
};
+#ifdef __cplusplus
+}
+#endif /* __cplusplus */
+
+#include <gerror.h>
+#ifdef __cplusplus
+extern "C" {
+#endif /* __cplusplus */
+
/* IEEE Standard 754 Single Precision Storage Format (gfloat):
*
* 31 30 23 22 0
@@ -3035,6 +3044,13 @@
/* GLib Thread support
*/
+#define G_THREAD_ERROR "GThread"
+
+typedef enum
+{
+ G_THREAD_ERROR_AGAIN /* Resource temporarily unavailable */
+} GThreadError;
+
typedef void (*GThreadFunc) (gpointer value);
typedef enum
@@ -3087,7 +3103,8 @@
gboolean joinable,
gboolean bound,
GThreadPriority priority,
- gpointer thread);
+ gpointer thread,
+ GError **error);
void (*thread_yield) (void);
void (*thread_join) (gpointer thread);
void (*thread_exit) (void);
@@ -3149,10 +3166,11 @@
gulong stack_size,
gboolean joinable,
gboolean bound,
- GThreadPriority priority);
+ GThreadPriority priority,
+ GError **error);
GThread* g_thread_self ();
-void g_thread_join (GThread* thread);
-void g_thread_set_priority (GThread* thread,
+void g_thread_join (GThread *thread);
+void g_thread_set_priority (GThread *thread,
GThreadPriority priority);
/* GStaticMutexes can be statically initialized with the value
@@ -3353,20 +3371,23 @@
gboolean bound,
GThreadPriority priority,
gboolean exclusive,
- gpointer user_data);
+ gpointer user_data,
+ GError **error);
/* Push new data into the thread pool. This task is assigned to a thread later
* (when the maximal number of threads is reached for that pool) or now
* (otherwise). If necessary a new thread will be started. The function
* returns immediatly */
void g_thread_pool_push (GThreadPool *pool,
- gpointer data);
+ gpointer data,
+ GError **error);
/* Set the number of threads, which can run concurrently for that pool, -1
* means no limit. 0 means has the effect, that the pool won't process
* requests until the limit is set higher again */
void g_thread_pool_set_max_threads (GThreadPool *pool,
- gint max_threads);
+ gint max_threads,
+ GError **error);
gint g_thread_pool_get_max_threads (GThreadPool *pool);
/* Get the number of threads assigned to that pool. This number doesn't
@@ -3398,6 +3419,5 @@
#endif /* __cplusplus */
#include <gunicode.h>
-#include <gerror.h>
#endif /* __G_LIB_H__ */
Index: gthread.c
===================================================================
RCS file: /cvs/gnome/glib/gthread.c,v
retrieving revision 1.6
diff -u -b -B -r1.6 gthread.c
--- gthread.c 2000/07/26 11:01:59 1.6
+++ gthread.c 2000/08/30 14:19:54
@@ -97,7 +97,7 @@
NULL, /* private_set */
(void(*)(GThreadFunc, gpointer, gulong,
gboolean, gboolean, GThreadPriority,
- gpointer))g_thread_fail, /* thread_create */
+ gpointer, GError**))g_thread_fail, /* thread_create */
NULL, /* thread_yield */
NULL, /* thread_join */
NULL, /* thread_exit */
@@ -381,10 +381,11 @@
gulong stack_size,
gboolean joinable,
gboolean bound,
- GThreadPriority priority)
+ GThreadPriority priority,
+ GError **error)
{
GRealThread* result = g_new (GRealThread, 1);
-
+ GError *local_error = NULL;
g_return_val_if_fail (thread_func, NULL);
result->thread.joinable = joinable;
@@ -394,10 +395,18 @@
result->arg = arg;
result->private_data = NULL;
G_LOCK (g_thread_create);
- G_THREAD_UF (thread_create, (g_thread_create_proxy, result, stack_size,
- joinable, bound, priority,
- &result->system_thread));
+ G_THREAD_UF (thread_create, (g_thread_create_proxy, result,
+ stack_size, joinable, bound, priority,
+ &result->system_thread, &local_error));
G_UNLOCK (g_thread_create);
+
+ if (local_error)
+ {
+ g_free (result);
+ g_propagate_error (error, local_error);
+ return NULL;
+ }
+
return (GThread*) result;
}
Index: gthreadpool.c
===================================================================
RCS file: /cvs/gnome/glib/gthreadpool.c,v
retrieving revision 1.2
diff -u -b -B -r1.2 gthreadpool.c
--- gthreadpool.c 2000/07/26 11:01:59 1.2
+++ gthreadpool.c 2000/08/30 14:19:54
@@ -55,7 +55,7 @@
static void g_thread_pool_free_internal (GRealThreadPool* pool);
static void g_thread_pool_thread_proxy (gpointer data);
-static void g_thread_pool_start_thread (GRealThreadPool* pool);
+static void g_thread_pool_start_thread (GRealThreadPool* pool, GError **error);
static void g_thread_pool_wakeup_and_stop_all (GRealThreadPool* pool);
#define g_thread_should_run(pool, len) \
@@ -162,7 +162,8 @@
}
static void
-g_thread_pool_start_thread (GRealThreadPool* pool)
+g_thread_pool_start_thread (GRealThreadPool *pool,
+ GError **error)
{
gboolean success = FALSE;
GThreadPriority priority = pool->pool.priority;
@@ -206,9 +207,19 @@
}
if (!success)
+ {
+ GError *local_error = NULL;
/* No thread was found, we have to start one new */
- g_thread_create (g_thread_pool_thread_proxy, pool, pool->pool.stack_size,
- FALSE, pool->pool.bound, priority);
+ g_thread_create (g_thread_pool_thread_proxy, pool,
+ pool->pool.stack_size, FALSE,
+ pool->pool.bound, priority, &local_error);
+
+ if (local_error)
+ {
+ g_propagate_error (error, local_error);
+ return;
+ }
+ }
/* See comment in g_thread_pool_thread_proxy as to why this is done
* here and not there */
@@ -222,7 +233,8 @@
gboolean bound,
GThreadPriority priority,
gboolean exclusive,
- gpointer user_data)
+ gpointer user_data,
+ GError **error)
{
GRealThreadPool *retval;
@@ -259,7 +271,18 @@
g_async_queue_lock (retval->queue);
while (retval->num_threads < retval->max_threads)
- g_thread_pool_start_thread (retval);
+ {
+ GError *local_error = NULL;
+ g_thread_pool_start_thread (retval, &local_error);
+ if (local_error)
+ {
+ g_propagate_error (error, local_error);
+ g_async_queue_unref_and_unlock (retval->queue);
+ g_free (retval);
+ /* FIXME: free all other threads */
+ return NULL;
+ }
+ }
g_async_queue_unlock (retval->queue);
}
@@ -269,7 +292,8 @@
void
g_thread_pool_push (GThreadPool *pool,
- gpointer data)
+ gpointer data,
+ GError **error)
{
GRealThreadPool *real = (GRealThreadPool*) pool;
@@ -286,15 +310,23 @@
if (!pool->exclusive && g_async_queue_length_unlocked (real->queue) >= 0)
{
/* No thread is waiting in the queue */
- g_thread_pool_start_thread (real);
+ GError *local_error = NULL;
+ g_thread_pool_start_thread (real, &local_error);
+ if (local_error)
+ {
+ g_propagate_error (error, local_error);
+ g_async_queue_unlock (real->queue);
+ return;
}
+ }
g_async_queue_push_unlocked (real->queue, data);
g_async_queue_unlock (real->queue);
}
void
g_thread_pool_set_max_threads (GThreadPool *pool,
- gint max_threads)
+ gint max_threads,
+ GError **error)
{
GRealThreadPool *real = (GRealThreadPool*) pool;
gint to_start;
@@ -314,7 +346,12 @@
to_start = g_async_queue_length_unlocked (real->queue);
for ( ; to_start > 0; to_start--)
- g_thread_pool_start_thread (real);
+ {
+ GError *local_error = NULL;
+ g_thread_pool_start_thread (real, &local_error);
+ if (local_error)
+ break;
+ }
g_async_queue_unlock (real->queue);
}
Index: gthread/gthread-posix.c
===================================================================
RCS file: /cvs/gnome/glib/gthread/gthread-posix.c,v
retrieving revision 1.17
diff -u -b -B -r1.17 gthread-posix.c
--- gthread/gthread-posix.c 2000/07/26 11:02:01 1.17
+++ gthread/gthread-posix.c 2000/08/30 14:19:54
@@ -232,9 +232,11 @@
gboolean joinable,
gboolean bound,
GThreadPriority priority,
- gpointer thread)
+ gpointer thread,
+ GError **error)
{
pthread_attr_t attr;
+ gint ret;
g_return_if_fail (thread_func);
@@ -274,11 +276,23 @@
# endif /* G_THREADS_IMPL_DCE */
#endif /* HAVE_PRIORITIES */
- posix_check_for_error (pthread_create (thread, &attr,
- (void* (*)(void*))thread_func,
- arg));
+ ret = pthread_create (thread, &attr, (void* (*)(void*))thread_func, arg);
+#ifdef G_THREADS_IMPL_DCE
+ if (ret == -1)
+ ret = errno;
+ else
+ ret = 0;
+#endif /* G_THREADS_IMPL_DCE */
+
posix_check_for_error (pthread_attr_destroy (&attr));
+
+ if (ret)
+ {
+ g_set_error (error, G_THREAD_ERROR, G_THREAD_ERROR_AGAIN,
+ "Error creating thread: %s", g_strerror (ret));
+ return;
+ }
#ifdef G_THREADS_IMPL_DCE
if (!joinable)
Index: tests/thread-test.c
===================================================================
RCS file: /cvs/gnome/glib/tests/thread-test.c,v
retrieving revision 1.3
diff -u -b -B -r1.3 thread-test.c
--- tests/thread-test.c 2000/04/19 09:29:19 1.3
+++ tests/thread-test.c 2000/08/30 14:19:54
@@ -27,7 +27,7 @@
g_assert (G_TRYLOCK (test_g_mutex));
thread = g_thread_create (test_g_mutex_thread,
GINT_TO_POINTER (42),
- 0, TRUE, TRUE, G_THREAD_PRIORITY_NORMAL);
+ 0, TRUE, TRUE, G_THREAD_PRIORITY_NORMAL, NULL);
g_usleep (G_MICROSEC);
test_g_mutex_int = 42;
G_UNLOCK (test_g_mutex);
@@ -62,7 +62,7 @@
g_assert (g_static_rec_mutex_trylock (&test_g_static_rec_mutex_mutex));
thread = g_thread_create (test_g_static_rec_mutex_thread,
GINT_TO_POINTER (42),
- 0, TRUE, TRUE, G_THREAD_PRIORITY_NORMAL);
+ 0, TRUE, TRUE, G_THREAD_PRIORITY_NORMAL, NULL);
g_usleep (G_MICROSEC);
g_assert (g_static_rec_mutex_trylock (&test_g_static_rec_mutex_mutex));
g_usleep (G_MICROSEC);
@@ -136,7 +136,7 @@
threads[i] = g_thread_create (test_g_static_private_thread,
GINT_TO_POINTER (i),
0, TRUE, TRUE,
- G_THREAD_PRIORITY_NORMAL);
+ G_THREAD_PRIORITY_NORMAL, NULL);
}
for (i = 0; i < THREADS; i++)
{
@@ -213,7 +213,7 @@
{
threads[i] = g_thread_create (test_g_static_rw_lock_thread,
0, 0, TRUE, TRUE,
- G_THREAD_PRIORITY_NORMAL);
+ G_THREAD_PRIORITY_NORMAL, NULL);
}
g_usleep (G_MICROSEC);
test_g_static_rw_lock_run = FALSE;
@@ -238,10 +238,21 @@
main (int argc,
char *argv[])
{
+ int i;
+ GError *err = NULL;
+ g_thread_init (NULL);
/* Only run the test, if threads are enabled and a default thread
implementation is available */
+ for (i = 0; i < 10000; i++)
+ {
+ g_thread_create (test_g_static_rw_lock_thread,
+ 0, 0, TRUE, TRUE,
+ G_THREAD_PRIORITY_NORMAL, &err);
+ g_print ("%d\n",i);
+ if (err)
+ g_error ("%s, %d, %s.\n", err->domain, err->code, err->message);
+ }
#if defined(G_THREADS_ENABLED) && ! defined(G_THREADS_IMPL_NONE)
- g_thread_init (NULL);
run_all_tests ();
/* Now we rerun all tests, but this time we fool the system into
Index: tests/threadpool-test.c
===================================================================
RCS file: /cvs/gnome/glib/tests/threadpool-test.c,v
retrieving revision 1.1
diff -u -b -B -r1.1 threadpool-test.c
--- tests/threadpool-test.c 2000/04/28 12:24:53 1.1
+++ tests/threadpool-test.c 2000/08/30 14:19:54
@@ -33,17 +33,17 @@
g_thread_init (NULL);
pool1 = g_thread_pool_new (thread_pool_func, 3, 0, FALSE,
- G_THREAD_PRIORITY_NORMAL, FALSE, NULL);
+ G_THREAD_PRIORITY_NORMAL, FALSE, NULL, NULL);
pool2 = g_thread_pool_new (thread_pool_func, 5, 0, FALSE,
- G_THREAD_PRIORITY_LOW, FALSE, NULL);
+ G_THREAD_PRIORITY_LOW, FALSE, NULL, NULL);
pool3 = g_thread_pool_new (thread_pool_func, 7, 0, FALSE,
- G_THREAD_PRIORITY_LOW, TRUE, NULL);
+ G_THREAD_PRIORITY_LOW, TRUE, NULL, NULL);
for (i = 0; i < RUNS; i++)
{
- g_thread_pool_push (pool1, GUINT_TO_POINTER (1));
- g_thread_pool_push (pool2, GUINT_TO_POINTER (1));
- g_thread_pool_push (pool3, GUINT_TO_POINTER (1));
+ g_thread_pool_push (pool1, GUINT_TO_POINTER (1), NULL);
+ g_thread_pool_push (pool2, GUINT_TO_POINTER (1), NULL);
+ g_thread_pool_push (pool3, GUINT_TO_POINTER (1), NULL);
}
g_thread_pool_free (pool1, FALSE, TRUE);
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
