Re: GError implementation

[Tim Janik <timj gtk org>]

On 23 Jun 2000, Havoc Pennington wrote:

> > >   gint         code;
> > >   const gchar *str;
> > 
> > can we write this out? "string" isn't an awfully long name, other
> > candidates are "description", or to think of the Gimp's API "blurb".
> > 
> 
> "str" is an established convention in glib, and "string" clashes with
> C++ in annoying ways (though it does compile, for example Emacs will
> syntax-highlight it specially). So I don't like it as much as str.

we usually write words out, at least something with less than 20
characters (i18n ;), GString->str breaks with taht and i stumbled
over it often enough, so i particularly dislike "str". it's just
personal ;) i get your c++ "string" rasoning though, so what about
"blurb"?

> > >   gpointer     data;
> > > };
> > > 
> > > #define g_error_matches(error,mod,num) (error && error->module == (mod) && error->code == (num))
> > 
> > why would a NULL pointer be a valid error?
> 
> Because you can write this:
> 
> GError *err = NULL;
> frobate (&err);
> if (g_error_matches (*err, BLAH_ERROR, BLAH_ERROR_FOO))
>   handle_error ();
> 
> otherwise you have to write this:
> 
> GError *err = NULL;
> frobate (&err);
> if (*err && g_error_matches (*err, BLAH_ERROR, BLAH_ERROR_FOO))
>   handle_error ();

oh bloody pointer hell, seeing that, i'd prefer:

GError error = G_ERROR_INIT;

frobate (&err);
if (g_error_matches (&err, BLAH_ERROR, BLAH_ERROR_FOO))
   handle_error (&err);

(of course then you need copying for passing around, but that
wouldn't be a problem if user_data is omitted).
iirc, this was one of your suggestions back in february,
it also has the addtional convenience benefit of having

g_error_set (GError *error, gchar *blurb)

actually operate on on a GError* pointer and you can also
allow NULL passing if the caller doesn't bother.

> And that's an easy thing to get wrong, and pointless typing.
> 
> >  usually you have error codes like
> > G_ERROR_OK=0 or G_ERROR_NONE=0. also it is a very bad idea to hide multiple
> > argument in a function alike macro, we use upper case macros for this:
> > G_ERROR_MATCHES() and even then try to avoid multiple argument evaluations.
> > for this specific case, i don't see why you couldn't use a normal function,
> > error code evaluation is rarely time critical, and even then, glib deals with
> > inline functions quite nicely.
> > 
> 
> Right, I'll make it a function.
>  
> > huh? ok there's no GDestroyNotify function in GError, probably an
> > omission,
> 
> It's in GRealError
> 
> > but what's that with GDataCopyFunc? where will you store that? 
> 
> 
> GRealError

urm, what benefits would GRealError provide?
you loose the ability to allocate errors on the stack, and clutter
your code with numerous pointless casts, reatrding your compiler's
check logic.
(yes, that's what i think about casts in general)

there's nothing wrong with

typedef struct
{
  GJam domain;
  gint code;
  gchar *blurb;
  gpointer data;
  /*< private >*/
  GDestroyNotify destroy;
} GError;

for people being anal and wanting pure encapsulation (omitting
any implications!) don't be half-ashamed and do use opaque
typedefs:

typedef struct _GError GError;
/* this is it for the public header file */

> > and btw,
> > you're pollution the already used GData namespace.
> 
> If we provide a long-term guarantee that GError is an immutable object
> (i.e. we can't add g_error_set_*()) then we could use ref/unref. If we
> reserve the right to make it mutable, we need a copy function.
> 
> If there's no way to copy or ref an error, people will get annoyed I'm
> sure, so we need one or the other.

right, i vote for a glib-provided copy function, doable by omitting
user_data.

> > also i'm so much in fond of proving printf semantics on the default
> > constructor, if you offer printf() semantics, you better make that explicit
> > in the API:
> > 
> > GError*  g_error_new           (GQuark         module,
> >                                 gint           code,
> >                                 gpointer       data,
> >                                 GDestroyNotify dnotify);
> > GError*  g_error_new_printf    (GQuark         module,
> >                                 gint           code,
> >                                 gpointer       data,
> >                                 GDestroyNotify dnotify,
> >                                 const gchar   *blurb,
> >                                 ...) G_GNUC_PRINTF (...);
> >
> 
> I think the printf version will be used 95% of the time, so it's kind
> of bad to make it long and inconvenient IMO.
> 
> Why not add a non-printf version:
> 
>   GError * g_error_new_literal    (GQuark, gint, const char *);

no, my point is more about making printf explicit.
since during programming i don't always look at
prototypes, so i prefer printf style functions to
contain "printf" in their name. guess great minds
think different here ;)

> > > void     g_error_set_with_data (GError       **err,
> > >                                 GQuark         module,
> > >                                 gint           code,
> > >                                 gpointer       data,
> > >                                 GDestroyNotify dnotify,
> > >                                 GDataCopyFunc  copyfunc,
> > >                                 const gchar   *format,
> > >                                 ...) G_GNUC_PRINTF (7, 8);
> > > 
> > > /* if (err && *err) { g_error_free(*err); *err = NULL; } */
> > > void     g_error_clear         (GError       **err);
> > 
> > oh hell, what's this about? ;)
> >
> 
> Just a convenience function...
> 
> g_error_set and g_error_clear aren't actually methods on GError, which
> is a bit confusing, in GConf I call them gconf_set_error() etc. But I
> don't like g_set_error () since it isn't in a glib sub-namespace...

put 'em on the stack ;)

> 
> Havoc
> 

---
ciaoTJ