Usually when sending an IQ we would want to have a reference to GabbleConnection in the callback. This means that GabbleConnection needs to be included almost every time in user_data either directly or through a structure. A more straightforward solution would have GabbleConnection be the source object for the async call and wrap wocky_porter_send_iq_async.
This branch adds utility functions for sending IQs. There are two alternative _finish functions, depending on whether you want the the XMPP error stanza, or an equivalent Telepathy error. It draws from Will's wocky_porter_send_iq_finish() function from his search-cleanup branch.
> + GError *error_ = NULL; Call it wocky_error, please. > +conn_util_send_iq_finish_harder (GabbleConnection *self, conn_util_send_iq_finish_with_tp_error? But I think I'd actually prefer a single finish function with two GError** parameters, one for the Wocky error and one for the Telepathized error (bonus points if you convert to TpError lazily).
oops, I missed this review earlier. (In reply to comment #2) > > + GError *error_ = NULL; > > Call it wocky_error, please. Got rid of this function. > > > +conn_util_send_iq_finish_harder (GabbleConnection *self, > > conn_util_send_iq_finish_with_tp_error? But I think I'd actually prefer a > single finish function with two GError** parameters, one for the Wocky error > and one for the Telepathized error (bonus points if you convert to TpError > lazily). The downside to that kind of finish function would be that you don't get the mutually exclusive NULL pattern with error and res_pointer (you could get an xmpp stanza, and also errors). Not sure what you mean by a lazy conversion, should I be checking for (GError **tp_error) != NULL before conversion? gabble_set_tp_error_from_wocky knows how to deal with that (although it probably could return sooner, and not pass to gabble_set_tp_conn_error_from_wocky if error == NULL). Anyway, I added this in d1094e8. I hope the lazy "||" is not too hackish for you!
(In reply to comment #3) > The downside to that kind of finish function would be that you don't get the > mutually exclusive NULL pattern with error and res_pointer (you could get an > xmpp stanza, and also errors). Right, that was the idea with my original _finish_harder(): you don't have to care about the stanza at all if it has type='error'. Simon, if you're super-super keen to only have one finish function, I would be in favour of making this function not return a stanza if an error was extracted from it. The advantage of having another function was that you could still use the _send() helper, even if you did want to do more interesting error handling. How about for now we claim YAGNI, and make the single _finish() function return a non-NULL WockyStanza if and only if the reply wasn't type='error', and have a single GError ** parameter? Eitan, I think this would work for your code using this function. We can revisit this later if we need to. > Not sure what you mean by a lazy conversion, should I be checking for (GError > **tp_error) != NULL before conversion? gabble_set_tp_error_from_wocky knows how > to deal with that (although it probably could return sooner, and not pass to > gabble_set_tp_conn_error_from_wocky if error == NULL). > > Anyway, I added this in d1094e8. I hope the lazy "||" is not too hackish for > you! I think this is lazy enough. If we wanted to make it lazier, the place to do that would be wocky_xmpp_error_extract(), which could short-circuit right after the 'if (type != NULL) {..}' block if none of its 'core', 'specialized', 'specialized_node' parameters are non-NULL.
(In reply to comment #4) > (In reply to comment #3) > > The downside to that kind of finish function would be that you don't get the > > mutually exclusive NULL pattern with error and res_pointer (you could get an > > xmpp stanza, and also errors). > > Right, that was the idea with my original _finish_harder(): you don't have to > care about the stanza at all if it has type='error'. > > Simon, if you're super-super keen to only have one finish function, I would be > in favour of making this function not return a stanza if an error was extracted > from it. The advantage of having another function was that you could still use > the _send() helper, even if you did want to do more interesting error handling. > > How about for now we claim YAGNI, and make the single _finish() function return > a non-NULL WockyStanza if and only if the reply wasn't type='error', and have a > single GError ** parameter? Eitan, I think this would work for your code using > this function. We can revisit this later if we need to. > The only useful thing to me in this recent change has been changing the finish function to a boolean, so that receiving a WockyStanza is not mandatory. I think a typical use is sending an IQ and expecting an empty ack result. No need to get the stanza for that.
(In reply to comment #5) > (In reply to comment #4) > > (In reply to comment #3) > > > The downside to that kind of finish function would be that you don't get the > > > mutually exclusive NULL pattern with error and res_pointer (you could get an > > > xmpp stanza, and also errors). > > > > Right, that was the idea with my original _finish_harder(): you don't have to > > care about the stanza at all if it has type='error'. > > > > Simon, if you're super-super keen to only have one finish function, I would be > > in favour of making this function not return a stanza if an error was extracted > > from it. The advantage of having another function was that you could still use > > the _send() helper, even if you did want to do more interesting error handling. > > > > How about for now we claim YAGNI, and make the single _finish() function return > > a non-NULL WockyStanza if and only if the reply wasn't type='error', and have a > > single GError ** parameter? Eitan, I think this would work for your code using > > this function. We can revisit this later if we need to. > > > > The only useful thing to me in this recent change has been changing the finish > function to a boolean, so that receiving a WockyStanza is not mandatory. I > think a typical use is sending an IQ and expecting an empty ack result. No need > to get the stanza for that. Pushed, 209fa2d
(In reply to comment #6) > Pushed, 209fa2d Forgot to free the GError, d43e83f.
r+ for master, thanks!
Thank, merged.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.