start of the new xmpp's connection

Review Request #2866 — Created Dec. 4, 2023 and submitted

Information

pidgin/pidgin
default

Reviewers

Right now this just establishes a TCP connection via SRV record and nothing
more.

Connected to pidginchat.com successfully and verified the traffic via charles proxy.

Summary ID
start of the new xmpp's connection
This uses XemeConnection to do all of the heavy lifting.
4e897d4341e0f24255330426bc164101842eeeef
Description From Last Updated

After a bunch of thought, it seems like moving the actual connection stuff into Xeme is the ideal way to …

grimgrim

Did you mean to do *error == NULL?

ivanhoeivanhoe

local_error must be non-NULL, which is not guaranteed by the condition above.

ivanhoeivanhoe

leak

ivanhoeivanhoe

This appears to need clearing in dispose or finalize?

QuLogicQuLogic

Need to clear resolver in here, as this condition could still be true with a resolver, but local_error != NULL.

QuLogicQuLogic

Space around ** in wrong spot.

QuLogicQuLogic
grim
grim
grim
grim
  1. 
      
  2. Show all issues

    After a bunch of thought, it seems like moving the actual connection stuff into Xeme is the ideal way to go because making everyone implement this is insane.

    I'll leave this open for now as reference, but by adding a XemeConnection object we can set properties and stuff on it from the account options and then call XemeConnection.connect_async to start the actual process and then obvious XemeConnection.connect_finish to get the results.

  3. 
      
grim
ivanhoe
  1. 
      
  2. protocols/xmpp/purplexmppconnection.c (Diff revision 5)
     
     
    Show all issues

    Did you mean to do *error == NULL?

  3. protocols/xmpp/purplexmppconnection.c (Diff revision 5)
     
     
    Show all issues

    local_error must be non-NULL, which is not guaranteed by the condition above.

  4. protocols/xmpp/purplexmppconnection.c (Diff revision 5)
     
     
    Show all issues

    leak

    1. This is cleared on line 150 right below. If you saw this leaking in valgrind it has to be in xeme and thus not related to this review request.

  5. 
      
grim
QuLogic
  1. 
      
  2. protocols/xmpp/purplexmppconnection.c (Diff revision 6)
     
     
    Show all issues

    This appears to need clearing in dispose or finalize?

  3. protocols/xmpp/purplexmppconnection.c (Diff revision 6)
     
     
    Show all issues

    Need to clear resolver in here, as this condition could still be true with a resolver, but local_error != NULL.

  4. protocols/xmpp/purplexmppprotocol.c (Diff revision 6)
     
     
    Show all issues

    Space around ** in wrong spot.

  5. 
      
grim
QuLogic
  1. Ship It!
  2. 
      
grim
ivanhoe
  1. Ship It!
  2. 
      
grim
Review request changed
Status:
Completed