On Mon, 2007-12-03 at 13:01 -0500, Dan Winship wrote: > Alexander Larsson wrote: > > On Thu, 2007-11-29 at 13:18 -0500, Dan Winship wrote: > >> It would be cleaner if I could just do: > >> > >> if (!g_input_stream_set_pending (stream, TRUE, error)) > >> return FALSE; > > > > Yeah, that would remove some duplication in other in-tree streams too. > > Care to hack up a patch for this? > > OK, three patches attached. The first gets rid of some cheating, where > code is currently temporarily clearing the "pending" flag so it can make > a nested call; the patch makes it use the class->foo method directly > rather than calling the wrapper methods. (This is what > g_input_stream_real_skip() already did, so it seemed to make sense to do > it elsewhere as well.)
I added an extra check for ->write != NULL in the splice case (because write() already did that) and commited. > Then the second patch implements > g_input_stream_set_pending/g_input_stream_clear_pending. I made the > set_pending method also check if the stream is already closed, since > everywhere we check for pending also checks for that (except > g_input_stream_close() and g_output_stream_close(), which handle that > case specially). I wondered if maybe it could also do > "g_push_current_cancellable()" (and be renamed to something more generic > at that point), but the async calls don't currently use > g_push_current_cancellable(), and I couldn't figure out what that was > even for anyway, so I didn't. This looks very nice. Commited. The push/pop cancellable are so that a sync call implementation does not have to carry with it the cancellable, but can instead use g_cancellable_get_current(). This is often needed in implementation that use some library where the design doesn't allow you to pass some data into the lowlevel read calls. > >> A related issue was that with soup_input_stream_send_async, I ended up > >> needing to have a wrapper callback to clear the pending flag before > >> calling the real callback, just like GInputStream does for its async > >> ops. Maybe GSimpleAsyncResult could support that idiom directly, by > >> providing an "implementation_callback" or whatever in addition to the > >> caller-provided callback. > > > > Yeah, that would be nice... Wanna hack? :) > > This ended up not being workable the way I suggested, because > g_input_stream_read_async, etc, don't have access to the > GSimpleAsyncResult, because it's not created until the _real_read_async > or whatever. But I realized we could still just do the cleanup in the > _finish methods rather than wrapping the callbacks, so I implemented > that, which is the third patch. > > The patch ended up a little ugly though, because each _finish method had > multiple exit points, so I needed to add lots of "goto done"s to ensure > the cleanup stuff always happened. Another possibility would be to do > the clear_pending and g_object_unref at the top of each _finish method, > relying on the fact that it's safe to keep using the stream after > unreffing it because the GAsyncResult is also holding a ref on it (since > it's the result's source_object). > > g_output_stream_splice_async() was also tricky because it needs to ref > and unref both the output stream and the source stream. In this patch I > fixed that by changing g_output_stream_splice_finish() to take the input > stream as a parameter as well, but maybe that's bad. I don't like this one. There is no guarantee that the _finish() call will be called. You might not care about the result, or it might be called multiple times. Also, for the splice case, the code that calls splice_finish() doesn't get passed the input stream, and it might not even need it. So it has to jump through extra hoops to keep it around just to pass it to _finish(). Seems bad to me. _______________________________________________ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list