Hi, I had an initial look at gvfs in particular the Inputstream and Outputstream implementations, and some comments came to my mind, in particular about the API. So I thought I'd post them as early as possible. These comments are about the code in http://www.gnome.org/~alexl/git/gvfs.git from today - May 2nd. I should also note that these ideas are probably heavily influenced by where I'm coming from - namely GStreamer and Swfdec - which involve handling low level byte streams possibly caring about low latency. In this particular case I had been wondering about making the basic input/output objects in Swfdec GInput/OutputStream.
1) I don't like GCancellable This is for several reasons: 1a) GCancellable is far too visible. It's present in every function call, so it must be very important. Maybe exporting g_push/pop_current_cancellable () would be a better API because it decouples the cancelling from the actual operations? 1b) Cancelling operations from another thread doesn't look like good design to me. I've learnt (both in theory and in practice) that threads are supposed to be independant and not call into each other. 2) GVFS seems to avoid the glib main loop for asynchronous operations in favour of relying on threads. I'm not sure this provides any benefits besides making apps way harder to debug. Is there a reason for that? 3) The whole API seems to be built around the model of synchronous operation. (I think so because of this code comment: /* Async ops: (optional in derived classes) */) I always thought everyone agrees that synchronous operation should be a thing of the past. and only be supported as an add-on for lazy coders or really simple apps. Almost everyone implements synchronous operations something like this: void foo () { while (errno == EAGAIN) foo_async (); }; The kernel sure does. 4) The asynchronous API seems to avoid the POSIX asynchronous model in favour of callbacks. Is there a reason why? I'd have guesses an asynchronous API looked like GIOChannel with some sugar on top. Something like this: if (!g_input_stream_read (stream, buf, size)) g_input_stream_wakeup_when_available (stream, size, func, data); And have that wake up in the glib main context when enough data is available. 5) You seem to use void * in as the data pointers. All applications I know use guchar * (some use gchar *) to handle data. From my stream handling experience this is to encourage people to think about what they pass to such a function. This seems to encourage calling functions like this: write (mywidget, sizeof (MyWidget)) - with is a bad idea for multiple reasons, including but not limited to struct padding. MS formats seem to have done that a lot. 6) Has there been thoughts about using a buffer-like struct? I seem to remember having talked about that last Guadec, but don't remember. To elaborate: A buffer structure is basically a refcounted memory region. All the multimedia libs make use of a structure like it. Some examples are at [1] and [2]. It allows some neat features like avoiding memcpys to reference the data and subbuffers. 7) The error handling in gvfs seems to be oriented towards the "old" and in glib still common error model of having people supply a GError to every function. I much prefer the cairo model, where basically the object keeps its own GError and has a function like cairo_status [3] that gives you the last error that occured. This has multiple advantages such as that I don't have to check for errors in every function, I can get the error on request and don't have to keep it around manually and function calls have one parameter less. 8) The API seems very forgiving to (IMO) obvious porogramming errors that should g_return_if_fail. g_input_stream_read for example provides a proper error message when there's already a pending operation or when the size parameter is too large. 9) I got the feeling that the API is somewhat designed to make one Stream object be usable from multiple threads at once. I can't pinpoint that, it's just a feeling. That's not the case, is it? Wow, that got more than I thought it would become. Please don't take that as me slamming gvfs, I'm just curious as to why those decisions were made. Cheers, Benjamin [1] http://swfdec.freedesktop.org/documentation/swfdec/swfdec-SwfdecBuffer.html [2] http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gstreamer/html/gstreamer-GstBuffer.html [3] http://www.cairographics.org/manual/cairo-cairo-t.html#cairo-status _______________________________________________ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list