Re: Some comments about GVFS
- From: Havoc Pennington <hp redhat com>
- To: Carl Worth <cworth redhat com>
- Cc: gtk-devel-list gnome org, Benjamin Otte <otte gnome org>
- Subject: Re: Some comments about GVFS
- Date: Thu, 03 May 2007 16:52:39 -0400
Carl Worth wrote:
Sure. If you want the object to remain usable after an error, then it
shouldn't shut down. And note that that with cairo, we do have
functions that do return an error indication instead of shutting down,
(cairo_surface_write_to_png, for example).
So I think a fair characterization of the "cairo model" is that it
recognizes that you don't always do the shutdown-on-error thing, but
it is darn handy when it's the right thing to do.
"cairo model" is probably confusing - write_to_png is a more typical and
essentially GError-equivalent model, while "cairo_t model" is the thing
that people mean is unique to Cairo.
For cairo, at least, there's nothing broken here. If you want to use
the same object from multiple threads, then you need to be doing your
Of course. The case where this would break is an object with its own
locks, e.g. DBusConnection. I'm just guessing that gvfs has its own
locks since I believe gnome-vfs did.
First and foremost: GError should only be used to report
recoverable runtime errors, never to report programming
errors. If the programmer has screwed up, then you should use
g_warning(), g_return_if_fail(), g_assert(), g_error(), or
some similar facility.
For that second sentence, if we're talking about a non-recoverable
programmer error, then what guidance is there for choosing
g_return_if_fail as opposed to g_assert? Are programmer errors
actually divided into two classes?
There is an official GLib/GTK+ policy on this, which is that g_assert is
used only for bugs in the library, and return_if_fail is used for bugs
in the app. It is missing from the docs indeed.
I could imagine a consistent argument that all programmer errors
should lead to an abort, (to force errors to get noticed and dealt
D-Bus in fact does this unless you disable it by env variable. It is
pretty controversial, though. Or at least there's a vocal group who
doesn't like it.
Obviously I think said group is wrong ;-)
But how does g_return_if_fail fit into your model? Isn't it really
doing basically the same thing as cairo's inert object functions?
Differences I see are that:
1. g_return_if_fail prints a message
2. cairo's inert objects come with a guarantee that once an
object shuts down, no future call to that object will have
The big differences are 1) that it prints a message and 2) that all
behavior is undefined after a return_if_fail - GTK doesn't contain
handling code that frees resources or tries to recover or puts objects
in a no-op state as a result of the return_if_fail.
I don't think 2) really matters to library users, it only affects
library maintainer effort.
So all you really want is a mode to make cairo be noisy on the first
Well, if and only if the error is a programming error. That's the
problem, I don't want noisy in write_to_png, nor do I want noisy if the
error is something I should be handling or is something I'm legitimately
ignoring. I only want noisy about bugs.
What D-Bus does in a couple of cases (and GTK may have one or two
examples of this also, iirc) is that if you pass NULL to ignore the
DBusError/GError, it is noisy, and if you don't pass NULL it is not
noisy. However, this is only OK if passing NULL when an error occurs
amounts to a programming bug, i.e. when you are only allowed to pass
NULL if you know errors are impossible. If it would be legitimate and
normal for the error to be silently ignored in some cases, you can't be
verbose when one happens. I can't actually find an example in the code
quickly, but I swear we do this in a couple places.
But a little noise isn't really enough information to tell you where
the error came from, so you're still going to have to track it
For example say you pass nonsense values; the warning can be
"function_name: assertion failed: arg_foo != 0.0" so I look at my calls
to function_name where arg_foo might be 0.0.
With cairo_t drawing code, you have to add temporary code to check the
status and print errors, then to figure out which function sets the
error you have to keep moving this temporary code around, then you have
to figure out what the status means once you know which function sets
the error status.
Setting a break on _cairo_error is certainly easier (thanks for that
tip) but the reason I've never done that is that I did not know cairo
had an internal symbol that was always used to set errors ;-) for the
analogous situation in gtk, gdk_x_error, the warning when an X error is
received has something in it like "try setting a break on gdk_x_error"
even, or used to.
What you'd really like is a nice application-level stack trace
from when the error gets detected by cairo, (hopefully people using
bindings are getting this already).
D-Bus does this on glibc systems - GNU libc has a function to get a
backtrace for you. So the warning includes the function, the line, the
assertion or precondition that failed, and a trace.
And here's the other big disagrement between you and me.
You're free to write applications that don't care about out of memory,
(most of mine don't---but I only write toys). But that's definitely
not a decision I'm willing to force on to all users of a system
library like cairo.
I don't disagree with you on this. D-Bus as another low-level library
also allows OOM handling.
I do think OOM handling would be insane in GTK+ purely because it would
be intractably hard, but for cairo it makes sense to me.
I think the cost of OOM handling in D-Bus is about a 30% code size
increase, and an absolute requirement to have substantial unit test
coverage that tests the OOM codepaths. For GTK+, I think it's an
intractable problem because the unit testing is intractable or at least
a monumental task. You could write the 30% code size but 10% of that
bloat would be broken and never get tested. And apps would never check
for OOM anyway.
In cairo I think ignoring OOM silently by default and allowing people to
check for it if they care is right.
So the fact that some users want to be able to recover from this and
some don't care is definitely a case where cairo's error-handling
strategy really helps.
Agreed. Please note, I don't have anything against the cairo_t model for
cairo_t (though I do wish it was not used for programming bugs, or at
least that optionally it was not used for programming bugs, or
optionally warning spam with function name, arg name, and assertion
failed could be printed for programming bugs). I only followed up to
point out that it isn't globally better and specifically I doubt it's
right for many things in gvfs.
I'll agree that the memory management of GError is a huge minus.
But it's also required to get an error message, unless you use globals
or happen to have a context object to stick the message to, like
cairo_t. If GError were only an error code it would have no management.
Also GError only has to be freed if an error in fact occurs, and most
GError-using functions also return a success/failure bool so if you
don't care about the specific error that happened, you can pass NULL to
avoid the GError and just check the return value.
But that would leave you with
always having g_assert and never g_return_if_fail for programming
errors though. So please explain more to me about how that handling
should be distinguished.
In GLib/GTK it's library bugs vs. app bugs, D-Bus maintains the same
distinction for _dbus_return_if_fail vs. _dbus_assert *but* D-Bus makes
them both fatal so it doesn't matter much anyhow.
] [Thread Prev