xmppo/go-xmpp

wrong idiom if &iq.Bind == nil. It's always not nil.

ikq opened this issue · 1 comments

ikq commented

Bind is struct in struct , so check &iq.Bind == nil is wrong, the sub-struct always have some address

var iq clientIQ
if &iq.Bind == nil {
  log.Printf("&iq.Bind is nil\n")
} else {
  log.Printf("&iq.Bind is not nil\n") // Always printed !
}

Proposed fix:
change in clientIQ definition Bind bindBind to Bind *bindBind, likes:

type clientIQ struct { // info/query
    XMLName xml.Name `xml:"jabber:client iq"`
    From    string   `xml:"from,attr"`
    ID      string   `xml:"id,attr"`
    To      string   `xml:"to,attr"`
    Type    string   `xml:"type,attr"` // error, get, result, set
    Error   *clientError  // Pointer to struct instead nested struct
    Bind    *bindBind    // Pointer...
}

And also change checking if the 'bind' field used to:

if iq.Bind == nil {
  return errors.New("<iq> result missing <bind>")
}

New version memory usage in Win32,
current version unsafe.Sizeof(clientIQ)=136
after my fix unsafe.Sizeof(clientIQ)=56 (in case <iq query...)
So if optional parts are not present, only nil pointer saved.

By the way, in the sruct changed tags:
previous version was xml:",attr", I use xml:"from,attr"

It's important because go xml parser is case sensitive and without the change, did not parse
from, to, type and id

I don't see any dereferencing of iq.Bind anymore in the code, so I'm going to close this.