diff mbox series

golang/xenlight: Fix handling of marshalling of empty elements for keyed unions

Message ID 20200306113939.693911-1-george.dunlap@citrix.com (mailing list archive)
State New, archived
Headers show
Series golang/xenlight: Fix handling of marshalling of empty elements for keyed unions | expand

Commit Message

George Dunlap March 6, 2020, 11:39 a.m. UTC
Keyed types in libxl_types.idl can have elements of type 'None'.  The
golang type generator (correctly) don't implement any union types for
these empty elements.  However, the toC and fromC helper generators
incorrectly treat these elements as invalid.

Consider for example, libxl_channelinfo.  The idl contains the
following keyed element:

    ("u", KeyedUnion(None, libxl_channel_connection, "connection",
           [("unknown", None),
            ("pty", Struct(None, [("path", string),])),
            ("socket", None),
           ])),

But the toC marshaller currently looks like this:

	switch x.Connection {
	case ChannelConnectionPty:
		tmp, ok := x.ConnectionUnion.(ChannelinfoConnectionUnionPty)
		if !ok {
			return errors.New("wrong type for union key connection")
		}
		var pty C.libxl_channelinfo_connection_union_pty
		if tmp.Path != "" {
			pty.path = C.CString(tmp.Path)
		}
		ptyBytes := C.GoBytes(unsafe.Pointer(&pty), C.sizeof_libxl_channelinfo_connection_union_pty)
		copy(xc.u[:], ptyBytes)
	default:
		return fmt.Errorf("invalid union key '%v'", x.Connection)
	}

Which means toC() will fail for ChannelConnectionUnknown or
ChannelConnectionSocket.

Modify the generator to handle keyed union elements of type 'None'.
For fromC, set the value to 'nil'; for toC, leave things as-is.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
---
In toC, instead of leaving the union alone for "empty" elements, we
could zero the whole union out.  That's probably not necessary.

CC: Ian Jackson <ian.jackson@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Nick Rosbrook <rosbrookn@ainfosec.com>
---
 tools/golang/xenlight/gengotypes.py  | 19 +++++---
 tools/golang/xenlight/helpers.gen.go | 66 ++++++++++++++++++++--------
 tools/golang/xenlight/types.gen.go   |  1 +
 3 files changed, 63 insertions(+), 23 deletions(-)

Comments

Nicholas Rosbrook March 6, 2020, 2:54 p.m. UTC | #1
> Keyed types in libxl_types.idl can have elements of type 'None'.  The
> golang type generator (correctly) don't implement any union types for
> these empty elements.  However, the toC and fromC helper generators
> incorrectly treat these elements as invalid.
>
> Consider for example, libxl_channelinfo.  The idl contains the
> following keyed element:
>
>     ("u", KeyedUnion(None, libxl_channel_connection, "connection",
>            [("unknown", None),
>             ("pty", Struct(None, [("path", string),])),
>             ("socket", None),
>            ])),
>
> But the toC marshaller currently looks like this:
> 
>         switch x.Connection {
>         case ChannelConnectionPty:
>                 tmp, ok := x.ConnectionUnion.(ChannelinfoConnectionUnionPty)
>                 if !ok {
>                         return errors.New("wrong type for union key connection")
>                 }
>                 var pty C.libxl_channelinfo_connection_union_pty
>                 if tmp.Path != "" {
>                         pty.path = C.CString(tmp.Path)
>                 }
>                 ptyBytes := C.GoBytes(unsafe.Pointer(&pty), C.sizeof_libxl_channelinfo_connection_union_pty)
>                 copy(xc.u[:], ptyBytes)
>         default:
>                 return fmt.Errorf("invalid union key '%v'", x.Connection)
>         }
>
> Which means toC() will fail for ChannelConnectionUnknown or
> ChannelConnectionSocket.
>
> Modify the generator to handle keyed union elements of type 'None'.
> For fromC, set the value to 'nil'; for toC, leave things as-is.
>
> Signed-off-by: George Dunlap <george.dunlap@citrix.com>

Reviewed-by: Nick Rosbrook <rosbrookn@ainfosec.com>
diff mbox series

Patch

diff --git a/tools/golang/xenlight/gengotypes.py b/tools/golang/xenlight/gengotypes.py
index f81271f3c0..50dada309b 100644
--- a/tools/golang/xenlight/gengotypes.py
+++ b/tools/golang/xenlight/gengotypes.py
@@ -335,6 +335,7 @@  def xenlight_golang_union_from_C(ty = None, union_name = '', struct_name = ''):
     gokeyname = xenlight_golang_fmt_name(keyname)
     keytype   = ty.keyvar.type.typename
     gokeytype = xenlight_golang_fmt_name(keytype)
+    field_name = xenlight_golang_fmt_name('{}_union'.format(keyname))
 
     interface_name = '{}_{}_union'.format(struct_name, keyname)
     interface_name = xenlight_golang_fmt_name(interface_name, exported=False)
@@ -351,11 +352,11 @@  def xenlight_golang_union_from_C(ty = None, union_name = '', struct_name = ''):
 
         # Add to list of cases to make for the switch
         # statement below.
+        cases[f.name] = (val, f.type)
+
         if f.type is None:
             continue
 
-        cases[f.name] = val
-
         # Define fromC func for 'union' struct.
         typename   = '{}_{}_union_{}'.format(struct_name,keyname,f.name)
         gotypename = xenlight_golang_fmt_name(typename)
@@ -382,9 +383,15 @@  def xenlight_golang_union_from_C(ty = None, union_name = '', struct_name = ''):
 
     # Create switch statement to determine which 'union element'
     # to populate in the Go struct.
-    for case_name, case_val in cases.items():
+    for case_name, case_tuple in cases.items():
+        (case_val, case_type) = case_tuple
+
         s += 'case {}:\n'.format(case_val)
 
+        if case_type is None:
+            s += "x.{} = nil\n".format(field_name)
+            continue
+
         gotype = '{}_{}_union_{}'.format(struct_name,keyname,case_name)
         gotype = xenlight_golang_fmt_name(gotype)
         goname = '{}_{}'.format(keyname,case_name)
@@ -394,7 +401,6 @@  def xenlight_golang_union_from_C(ty = None, union_name = '', struct_name = ''):
         s += 'if err := {}.fromC(xc);'.format(goname)
         s += 'err != nil {{\n return fmt.Errorf("converting field {}: %v", err) \n}}\n'.format(goname)
 
-        field_name = xenlight_golang_fmt_name('{}_union'.format(keyname))
         s += 'x.{} = {}\n'.format(field_name, goname)
 
     # End switch statement
@@ -551,10 +557,13 @@  def xenlight_golang_union_to_C(ty = None, union_name = '',
     for f in ty.fields:
         key_val = '{}_{}'.format(keytype, f.name)
         key_val = xenlight_golang_fmt_name(key_val)
+
+        s += 'case {}:\n'.format(key_val)
+
         if f.type is None:
+            s += "break\n"
             continue
 
-        s += 'case {}:\n'.format(key_val)
         cgotype = '{}_{}_union_{}'.format(struct_name,keyname,f.name)
         gotype  = xenlight_golang_fmt_name(cgotype)
 
diff --git a/tools/golang/xenlight/helpers.gen.go b/tools/golang/xenlight/helpers.gen.go
index b4d7dca1c6..344ce9a461 100644
--- a/tools/golang/xenlight/helpers.gen.go
+++ b/tools/golang/xenlight/helpers.gen.go
@@ -493,12 +493,16 @@  func (x *Channelinfo) fromC(xc *C.libxl_channelinfo) error {
 	x.Rref = int(xc.rref)
 	x.Connection = ChannelConnection(xc.connection)
 	switch x.Connection {
+	case ChannelConnectionUnknown:
+		x.ConnectionUnion = nil
 	case ChannelConnectionPty:
 		var connectionPty ChannelinfoConnectionUnionPty
 		if err := connectionPty.fromC(xc); err != nil {
 			return fmt.Errorf("converting field connectionPty: %v", err)
 		}
 		x.ConnectionUnion = connectionPty
+	case ChannelConnectionSocket:
+		x.ConnectionUnion = nil
 	default:
 		return fmt.Errorf("invalid union key '%v'", x.Connection)
 	}
@@ -537,6 +541,8 @@  func (x *Channelinfo) toC(xc *C.libxl_channelinfo) (err error) {
 	xc.rref = C.int(x.Rref)
 	xc.connection = C.libxl_channel_connection(x.Connection)
 	switch x.Connection {
+	case ChannelConnectionUnknown:
+		break
 	case ChannelConnectionPty:
 		tmp, ok := x.ConnectionUnion.(ChannelinfoConnectionUnionPty)
 		if !ok {
@@ -548,6 +554,8 @@  func (x *Channelinfo) toC(xc *C.libxl_channelinfo) (err error) {
 		}
 		ptyBytes := C.GoBytes(unsafe.Pointer(&pty), C.sizeof_libxl_channelinfo_connection_union_pty)
 		copy(xc.u[:], ptyBytes)
+	case ChannelConnectionSocket:
+		break
 	default:
 		return fmt.Errorf("invalid union key '%v'", x.Connection)
 	}
@@ -701,6 +709,7 @@  func (x *DomainCreateInfo) fromC(xc *C.libxl_domain_create_info) error {
 	x.Ssidref = uint32(xc.ssidref)
 	x.SsidLabel = C.GoString(xc.ssid_label)
 	x.Name = C.GoString(xc.name)
+	x.Domid = Domid(xc.domid)
 	if err := x.Uuid.fromC(&xc.uuid); err != nil {
 		return fmt.Errorf("converting field Uuid: %v", err)
 	}
@@ -744,6 +753,7 @@  func (x *DomainCreateInfo) toC(xc *C.libxl_domain_create_info) (err error) {
 	if x.Name != "" {
 		xc.name = C.CString(x.Name)
 	}
+	xc.domid = C.libxl_domid(x.Domid)
 	if err := x.Uuid.toC(&xc.uuid); err != nil {
 		return fmt.Errorf("converting field Uuid: %v", err)
 	}
@@ -1203,24 +1213,26 @@  func (x *DomainBuildInfo) fromC(xc *C.libxl_domain_build_info) error {
 	x.Tee = TeeType(xc.tee)
 	x.Type = DomainType(xc._type)
 	switch x.Type {
-	case DomainTypePv:
-		var typePv DomainBuildInfoTypeUnionPv
-		if err := typePv.fromC(xc); err != nil {
-			return fmt.Errorf("converting field typePv: %v", err)
-		}
-		x.TypeUnion = typePv
 	case DomainTypeHvm:
 		var typeHvm DomainBuildInfoTypeUnionHvm
 		if err := typeHvm.fromC(xc); err != nil {
 			return fmt.Errorf("converting field typeHvm: %v", err)
 		}
 		x.TypeUnion = typeHvm
+	case DomainTypePv:
+		var typePv DomainBuildInfoTypeUnionPv
+		if err := typePv.fromC(xc); err != nil {
+			return fmt.Errorf("converting field typePv: %v", err)
+		}
+		x.TypeUnion = typePv
 	case DomainTypePvh:
 		var typePvh DomainBuildInfoTypeUnionPvh
 		if err := typePvh.fromC(xc); err != nil {
 			return fmt.Errorf("converting field typePvh: %v", err)
 		}
 		x.TypeUnion = typePvh
+	case DomainTypeInvalid:
+		x.TypeUnion = nil
 	default:
 		return fmt.Errorf("invalid union key '%v'", x.Type)
 	}
@@ -1721,6 +1733,8 @@  func (x *DomainBuildInfo) toC(xc *C.libxl_domain_build_info) (err error) {
 		}
 		pvhBytes := C.GoBytes(unsafe.Pointer(&pvh), C.sizeof_libxl_domain_build_info_type_union_pvh)
 		copy(xc.u[:], pvhBytes)
+	case DomainTypeInvalid:
+		break
 	default:
 		return fmt.Errorf("invalid union key '%v'", x.Type)
 	}
@@ -2648,6 +2662,10 @@  func (x *DeviceChannel) fromC(xc *C.libxl_device_channel) error {
 	x.Name = C.GoString(xc.name)
 	x.Connection = ChannelConnection(xc.connection)
 	switch x.Connection {
+	case ChannelConnectionUnknown:
+		x.ConnectionUnion = nil
+	case ChannelConnectionPty:
+		x.ConnectionUnion = nil
 	case ChannelConnectionSocket:
 		var connectionSocket DeviceChannelConnectionUnionSocket
 		if err := connectionSocket.fromC(xc); err != nil {
@@ -2688,6 +2706,10 @@  func (x *DeviceChannel) toC(xc *C.libxl_device_channel) (err error) {
 	}
 	xc.connection = C.libxl_channel_connection(x.Connection)
 	switch x.Connection {
+	case ChannelConnectionUnknown:
+		break
+	case ChannelConnectionPty:
+		break
 	case ChannelConnectionSocket:
 		tmp, ok := x.ConnectionUnion.(DeviceChannelConnectionUnionSocket)
 		if !ok {
@@ -4368,24 +4390,28 @@  func (x *Event) fromC(xc *C.libxl_event) error {
 	x.ForUser = uint64(xc.for_user)
 	x.Type = EventType(xc._type)
 	switch x.Type {
-	case EventTypeOperationComplete:
-		var typeOperationComplete EventTypeUnionOperationComplete
-		if err := typeOperationComplete.fromC(xc); err != nil {
-			return fmt.Errorf("converting field typeOperationComplete: %v", err)
-		}
-		x.TypeUnion = typeOperationComplete
 	case EventTypeDomainShutdown:
 		var typeDomainShutdown EventTypeUnionDomainShutdown
 		if err := typeDomainShutdown.fromC(xc); err != nil {
 			return fmt.Errorf("converting field typeDomainShutdown: %v", err)
 		}
 		x.TypeUnion = typeDomainShutdown
+	case EventTypeDomainDeath:
+		x.TypeUnion = nil
 	case EventTypeDiskEject:
 		var typeDiskEject EventTypeUnionDiskEject
 		if err := typeDiskEject.fromC(xc); err != nil {
 			return fmt.Errorf("converting field typeDiskEject: %v", err)
 		}
 		x.TypeUnion = typeDiskEject
+	case EventTypeOperationComplete:
+		var typeOperationComplete EventTypeUnionOperationComplete
+		if err := typeOperationComplete.fromC(xc); err != nil {
+			return fmt.Errorf("converting field typeOperationComplete: %v", err)
+		}
+		x.TypeUnion = typeOperationComplete
+	case EventTypeDomainCreateConsoleAvailable:
+		x.TypeUnion = nil
 	default:
 		return fmt.Errorf("invalid union key '%v'", x.Type)
 	}
@@ -4452,6 +4478,8 @@  func (x *Event) toC(xc *C.libxl_event) (err error) {
 		domain_shutdown.shutdown_reason = C.uint8_t(tmp.ShutdownReason)
 		domain_shutdownBytes := C.GoBytes(unsafe.Pointer(&domain_shutdown), C.sizeof_libxl_event_type_union_domain_shutdown)
 		copy(xc.u[:], domain_shutdownBytes)
+	case EventTypeDomainDeath:
+		break
 	case EventTypeDiskEject:
 		tmp, ok := x.TypeUnion.(EventTypeUnionDiskEject)
 		if !ok {
@@ -4475,6 +4503,8 @@  func (x *Event) toC(xc *C.libxl_event) (err error) {
 		operation_complete.rc = C.int(tmp.Rc)
 		operation_completeBytes := C.GoBytes(unsafe.Pointer(&operation_complete), C.sizeof_libxl_event_type_union_operation_complete)
 		copy(xc.u[:], operation_completeBytes)
+	case EventTypeDomainCreateConsoleAvailable:
+		break
 	default:
 		return fmt.Errorf("invalid union key '%v'", x.Type)
 	}
@@ -4545,18 +4575,18 @@  func (x *PsrHwInfo) fromC(xc *C.libxl_psr_hw_info) error {
 	x.Id = uint32(xc.id)
 	x.Type = PsrFeatType(xc._type)
 	switch x.Type {
-	case PsrFeatTypeMba:
-		var typeMba PsrHwInfoTypeUnionMba
-		if err := typeMba.fromC(xc); err != nil {
-			return fmt.Errorf("converting field typeMba: %v", err)
-		}
-		x.TypeUnion = typeMba
 	case PsrFeatTypeCat:
 		var typeCat PsrHwInfoTypeUnionCat
 		if err := typeCat.fromC(xc); err != nil {
 			return fmt.Errorf("converting field typeCat: %v", err)
 		}
 		x.TypeUnion = typeCat
+	case PsrFeatTypeMba:
+		var typeMba PsrHwInfoTypeUnionMba
+		if err := typeMba.fromC(xc); err != nil {
+			return fmt.Errorf("converting field typeMba: %v", err)
+		}
+		x.TypeUnion = typeMba
 	default:
 		return fmt.Errorf("invalid union key '%v'", x.Type)
 	}
diff --git a/tools/golang/xenlight/types.gen.go b/tools/golang/xenlight/types.gen.go
index ede49b4886..4aaee20b95 100644
--- a/tools/golang/xenlight/types.gen.go
+++ b/tools/golang/xenlight/types.gen.go
@@ -403,6 +403,7 @@  type DomainCreateInfo struct {
 	Ssidref           uint32
 	SsidLabel         string
 	Name              string
+	Domid             Domid
 	Uuid              Uuid
 	Xsdata            KeyValueList
 	Platformdata      KeyValueList