opcua icon indicating copy to clipboard operation
opcua copied to clipboard

Should extension objects always be pointers?

Open magiconair opened this issue 1 year ago • 1 comments

I am wondering whether we should notify the user if the extension object is not a pointer or if the ua.NewExtensionObject should return an error (breaking change).

Usually, you register an extension object with new(X) and then create an instance with ua.NewExtensionObject(&X{}). However, ua.NewExtensionObject(X{}) is also valid but will not set the type id since *X is registered but X isn't.

We just spent a lot of time trying to find that problem. We have a couple of ways to fix this

  1. Change ua.NewExtensionObject to return an error
  2. Make the Lookup method check for value types and then look for pointer types
  3. panic if ua.NewExtensionObject does not get a pointer

What does the community think?

CC: @kung-foo @dwhutchison @alexbrdn

Example

package main

import (
    "fmt"

    "github.com/gopcua/opcua"
)

type X struct {}

func main() {
    ua.RegisterExtensionObject(ua.NewStringNodeID(1, "X"), new(X))
    eo := ua.NewExtensionObject(X{})
    fmt.Println(eo.TypeID) // prints i=0 instead of ns=1;s=X !!!
}

magiconair avatar Aug 10 '22 10:08 magiconair

I would prefer a panic over unexpected behavior. On the other hand, panicking in a lib is bad practice. So modifying NewExtensionObject to return an error is probably best.

Maybe RegisterExtensionObject should return a factory function for making new instances?

kung-foo avatar Aug 11 '22 09:08 kung-foo