Fix binapigen decoding and minor improvements

- fixed allocating byte slices before copying decoded data
- simplified encoding functions
- several minor improvements

Change-Id: I6669424b89eb86333805cb1b57e4551169980cc2
Signed-off-by: Ondrej Fabry <ofabry@cisco.com>

11 files changed, 342 insertions, 287 deletions

diff --git a/binapigen/binapigen.go b/binapigen/binapigen.go
index 1b4c7e5..2dbd661 100644
--- a/binapigen/binapigen.go
+++ b/binapigen/binapigen.go
@@ -17,6 +17,7 @@ package binapigen
 import (
 	"fmt"
 	"path"
+	"strconv"
 	"strings"
 
 	"git.fd.io/govpp.git/binapigen/vppapi"
@@ -154,14 +155,6 @@ func (file *File) dependsOnFile(gen *Generator, dep string) bool {
 	return false
 }
 
-func normalizeImport(imp string) string {
-	imp = path.Base(imp)
-	if idx := strings.Index(imp, "."); idx >= 0 {
-		imp = imp[:idx]
-	}
-	return imp
-}
-
 const (
 	enumFlagSuffix = "_flags"
 )
@@ -251,8 +244,7 @@ func newStruct(gen *Generator, file *File, apitype vppapi.StructType) *Struct {
 	}
 	gen.structsByName[typ.Name] = typ
 	for _, fieldType := range apitype.Fields {
-		field := newField(gen, file, fieldType)
-		field.ParentStruct = typ
+		field := newField(gen, file, typ, fieldType)
 		typ.Fields = append(typ.Fields, field)
 	}
 	return typ
@@ -285,8 +277,7 @@ func newUnion(gen *Generator, file *File, apitype vppapi.UnionType) *Union {
 	}
 	gen.unionsByName[typ.Name] = typ
 	for _, fieldType := range apitype.Fields {
-		field := newField(gen, file, fieldType)
-		field.ParentUnion = typ
+		field := newField(gen, file, typ, fieldType)
 		typ.Fields = append(typ.Fields, field)
 	}
 	return typ
@@ -311,25 +302,11 @@ const (
 	msgTypeEvent                  // msg_id, client_index
 )
 
-func apiMsgType(t msgType) GoIdent {
-	switch t {
-	case msgTypeRequest:
-		return govppApiPkg.Ident("RequestMessage")
-	case msgTypeReply:
-		return govppApiPkg.Ident("ReplyMessage")
-	case msgTypeEvent:
-		return govppApiPkg.Ident("EventMessage")
-	default:
-		return govppApiPkg.Ident("OtherMessage")
-	}
-}
-
 // message fields
 const (
 	fieldMsgID       = "_vl_msg_id"
 	fieldClientIndex = "client_index"
 	fieldContext     = "context"
-	fieldRetval      = "retval"
 )
 
 // field options
@@ -356,20 +333,17 @@ func newMessage(gen *Generator, file *File, apitype vppapi.Message) *Message {
 		GoIdent: newGoIdent(file, apitype.Name),
 	}
 	gen.messagesByName[apitype.Name] = msg
-	n := 0
+	var n int
 	for _, fieldType := range apitype.Fields {
-		// skip internal fields
-		switch strings.ToLower(fieldType.Name) {
-		case fieldMsgID:
-			continue
-		case fieldClientIndex, fieldContext:
-			if n == 0 {
+		if n == 0 {
+			// skip header fields
+			switch strings.ToLower(fieldType.Name) {
+			case fieldMsgID, fieldClientIndex, fieldContext:
 				continue
 			}
 		}
 		n++
-		field := newField(gen, file, fieldType)
-		field.ParentMessage = msg
+		field := newField(gen, file, msg, fieldType)
 		msg.Fields = append(msg.Fields, field)
 	}
 	return msg
@@ -389,16 +363,17 @@ func (m *Message) resolveDependencies(gen *Generator) (err error) {
 
 func getMsgType(m vppapi.Message) (msgType, error) {
 	if len(m.Fields) == 0 {
-		return msgType(0), fmt.Errorf("message %s has no fields", m.Name)
+		return msgType(-1), fmt.Errorf("message %s has no fields", m.Name)
 	}
-	typ := msgTypeBase
-	wasClientIndex := false
+	var typ msgType
+	var wasClientIndex bool
 	for i, field := range m.Fields {
-		if i == 0 {
+		switch i {
+		case 0:
 			if field.Name != fieldMsgID {
-				return msgType(0), fmt.Errorf("message %s is missing ID field", m.Name)
+				return msgType(-1), fmt.Errorf("message %s is missing ID field", m.Name)
 			}
-		} else if i == 1 {
+		case 1:
 			if field.Name == fieldClientIndex {
 				// "client_index" as the second member,
 				// this might be an event message or a request
@@ -408,8 +383,8 @@ func getMsgType(m vppapi.Message) (msgType, error) {
 				// reply needs "context" as the second member
 				typ = msgTypeReply
 			}
-		} else if i == 2 {
-			if wasClientIndex && field.Name == fieldContext {
+		case 2:
+			if field.Name == fieldContext && wasClientIndex {
 				// request needs "client_index" as the second member
 				// and "context" as the third member
 				typ = msgTypeRequest
@@ -419,34 +394,50 @@ func getMsgType(m vppapi.Message) (msgType, error) {
 	return typ, nil
 }
 
+// Field represents a field for message or struct/union types.
 type Field struct {
 	vppapi.Field
 
 	GoName string
 
+	// DefaultValue is a default value of field or
+	// nil if default value is not defined for field.
 	DefaultValue interface{}
 
-	// Reference to actual type of this field
+	// Reference to actual type of this field.
+	//
+	// For fields with built-in types all of these are nil,
+	// otherwise only one is set to non-nil value.
 	TypeEnum   *Enum
 	TypeAlias  *Alias
 	TypeStruct *Struct
 	TypeUnion  *Union
 
-	// Parent in which this field is declared
+	// Parent in which this field is declared.
 	ParentMessage *Message
 	ParentStruct  *Struct
 	ParentUnion   *Union
 
-	// Field reference for fields determining size
+	// Field reference for fields with variable size.
 	FieldSizeOf   *Field
 	FieldSizeFrom *Field
 }
 
-func newField(gen *Generator, file *File, apitype vppapi.Field) *Field {
+func newField(gen *Generator, file *File, parent interface{}, apitype vppapi.Field) *Field {
 	typ := &Field{
 		Field:  apitype,
 		GoName: camelCaseName(apitype.Name),
 	}
+	switch p := parent.(type) {
+	case *Message:
+		typ.ParentMessage = p
+	case *Struct:
+		typ.ParentStruct = p
+	case *Union:
+		typ.ParentUnion = p
+	default:
+		panic(fmt.Sprintf("invalid field parent: %T", parent))
+	}
 	if apitype.Meta != nil {
 		if val, ok := apitype.Meta[optFieldDefault]; ok {
 			typ.DefaultValue = val
@@ -578,3 +569,61 @@ func (rpc *RPC) resolveMessages(gen *Generator) error {
 	}
 	return nil
 }
+
+// GoIdent is a Go identifier, consisting of a name and import path.
+// The name is a single identifier and may not be a dot-qualified selector.
+type GoIdent struct {
+	GoName       string
+	GoImportPath GoImportPath
+}
+
+func (id GoIdent) String() string {
+	return fmt.Sprintf("%q.%v", id.GoImportPath, id.GoName)
+}
+
+func newGoIdent(f *File, fullName string) GoIdent {
+	name := strings.TrimPrefix(fullName, string(f.PackageName)+".")
+	return GoIdent{
+		GoName:       camelCaseName(name),
+		GoImportPath: f.GoImportPath,
+	}
+}
+
+// GoImportPath is a Go import path for a package.
+type GoImportPath string
+
+func (p GoImportPath) String() string {
+	return strconv.Quote(string(p))
+}
+
+func (p GoImportPath) Ident(s string) GoIdent {
+	return GoIdent{GoName: s, GoImportPath: p}
+}
+
+type GoPackageName string
+
+func cleanPackageName(name string) GoPackageName {
+	return GoPackageName(sanitizedName(name))
+}
+
+// baseName returns the last path element of the name, with the last dotted suffix removed.
+func baseName(name string) string {
+	// First, find the last element
+	if i := strings.LastIndex(name, "/"); i >= 0 {
+		name = name[i+1:]
+	}
+	// Now drop the suffix
+	if i := strings.LastIndex(name, "."); i >= 0 {
+		name = name[:i]
+	}
+	return name
+}
+
+// normalizeImport returns the last path element of the import, with all dotted suffixes removed.
+func normalizeImport(imp string) string {
+	imp = path.Base(imp)
+	if idx := strings.Index(imp, "."); idx >= 0 {
+		imp = imp[:idx]
+	}
+	return imp
+}
diff --git a/binapigen/binapigen_test.go b/binapigen/binapigen_test.go
index 2fbd163..9a25420 100644
--- a/binapigen/binapigen_test.go
+++ b/binapigen/binapigen_test.go
@@ -53,3 +53,20 @@ func TestGenerator(t *testing.T) {
 		})
 	}
 }
+
+func TestSanitize(t *testing.T) {
+	tests := []struct {
+		name     string
+		expected string
+	}{
+		{"interface", "interfaces"},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			s := sanitizedName(test.name)
+			if s != test.expected {
+				t.Fatalf("expected: %q, got: %q", test.expected, s)
+			}
+		})
+	}
+}
diff --git a/binapigen/gen_encoding.go b/binapigen/gen_encoding.go
index 1cd3eb3..d946771 100644
--- a/binapigen/gen_encoding.go
+++ b/binapigen/gen_encoding.go
@@ -17,7 +17,6 @@ package binapigen
 import (
 	"fmt"
 	"strconv"
-	"strings"
 
 	"github.com/sirupsen/logrus"
 )
@@ -26,10 +25,9 @@ func init() {
 	//RegisterPlugin("encoding", GenerateEncoding)
 }
 
-func generateMessageSize(g *GenFile, name string, fields []*Field) {
-	g.P("func (m *", name, ") Size() int {")
+func genMessageSize(g *GenFile, name string, fields []*Field) {
+	g.P("func (m *", name, ") Size() (size int) {")
 	g.P("if m == nil { return 0 }")
-	g.P("var size int")
 
 	sizeBaseType := func(typ, name string, length int, sizefrom string) {
 		switch typ {
@@ -77,17 +75,21 @@ func generateMessageSize(g *GenFile, name string, fields []*Field) {
 			}
 
 			if field.Array {
-				char := fmt.Sprintf("s%d", lvl)
 				index := fmt.Sprintf("j%d", lvl)
 				if field.Length > 0 {
 					g.P("for ", index, " := 0; ", index, " < ", field.Length, "; ", index, "++ {")
 				} else if field.FieldSizeFrom != nil {
 					g.P("for ", index, " := 0; ", index, " < len(", name, "); ", index, "++ {")
 				}
-				g.P("var ", char, " ", fieldGoType(g, field))
-				g.P("_ = ", char)
-				g.P("if ", index, " < len(", name, ") { ", char, " = ", name, "[", index, "] }")
-				name = char
+				if field.Length == 0 || field.SizeFrom != "" {
+					char := fmt.Sprintf("s%d", lvl)
+					g.P("var ", char, " ", fieldGoType(g, field))
+					g.P("_ = ", char)
+					g.P("if ", index, " < len(", name, ") { ", char, " = ", name, "[", index, "] }")
+					name = char
+				} else {
+					name = fmt.Sprintf("%s[%s]", name, index)
+				}
 			}
 
 			switch {
@@ -127,44 +129,17 @@ func generateMessageSize(g *GenFile, name string, fields []*Field) {
 	g.P("}")
 }
 
-func encodeBaseType(g *GenFile, typ, name string, length int, sizefrom string) {
-	isArray := length > 0 || sizefrom != ""
-	if isArray {
-		switch typ {
-		case U8:
-			g.P("buf.EncodeBytes(", name, "[:], ", length, ")")
-			return
-		case I8, I16, U16, I32, U32, I64, U64, F64:
-			gotype := BaseTypesGo[typ]
-			if length != 0 {
-				g.P("for i := 0; i < ", length, "; i++ {")
-			} else if sizefrom != "" {
-				g.P("for i := 0; i < len(", name, "); i++ {")
-			}
-			g.P("var x ", gotype)
-			g.P("if i < len(", name, ") { x = ", gotype, "(", name, "[i]) }")
-			name = "x"
-		}
-	}
-	switch typ {
-	case I8, U8, I16, U16, I32, U32, I64, U64:
-		typsize := BaseTypeSizes[typ]
-		g.P("buf.EncodeUint", typsize*8, "(uint", typsize*8, "(", name, "))")
-	case F64:
-		g.P("buf.EncodeFloat64(float64(", name, "))")
-	case BOOL:
-		g.P("buf.EncodeBool(", name, ")")
-	case STRING:
-		g.P("buf.EncodeString(", name, ", ", length, ")")
-	default:
-		logrus.Panicf("// ??? %s %s\n", name, typ)
-	}
-	if isArray {
-		switch typ {
-		case I8, U8, I16, U16, I32, U32, I64, U64, F64:
-			g.P("}")
-		}
-	}
+func genMessageMarshal(g *GenFile, name string, fields []*Field) {
+	g.P("func (m *", name, ") Marshal(b []byte) ([]byte, error) {")
+	g.P("if b == nil {")
+	g.P("b = make([]byte, m.Size())")
+	g.P("}")
+	g.P("buf := ", govppCodecPkg.Ident("NewBuffer"), "(b)")
+
+	encodeFields(g, fields, "m", 0)
+
+	g.P("return buf.Bytes(), nil")
+	g.P("}")
 }
 
 func encodeFields(g *GenFile, fields []*Field, parentName string, lvl int) {
@@ -182,7 +157,8 @@ func encodeFields(g *GenFile, fields []*Field, parentName string, lvl int) {
 func encodeField(g *GenFile, field *Field, name string, getFieldName func(name string) string, lvl int) {
 	if f := field.FieldSizeOf; f != nil {
 		if _, ok := BaseTypesGo[field.Type]; ok {
-			encodeBaseType(g, field.Type, fmt.Sprintf("len(%s)", getFieldName(f.GoName)), field.Length, "")
+			val := fmt.Sprintf("len(%s)", getFieldName(f.GoName))
+			encodeBaseType(g, field.Type, "int", val, 0, "", false)
 			return
 		} else {
 			panic(fmt.Sprintf("failed to encode base type of sizefrom field: %s (%s)", field.Name, field.Type))
@@ -194,37 +170,46 @@ func encodeField(g *GenFile, field *Field, name string, getFieldName func(name s
 	}
 
 	if _, ok := BaseTypesGo[field.Type]; ok {
-		encodeBaseType(g, field.Type, name, field.Length, sizeFromName)
+		encodeBaseType(g, field.Type, fieldGoType(g, field), name, field.Length, sizeFromName, true)
 		return
 	}
 
 	if field.Array {
-		char := fmt.Sprintf("v%d", lvl)
 		index := fmt.Sprintf("j%d", lvl)
 		if field.Length > 0 {
 			g.P("for ", index, " := 0; ", index, " < ", field.Length, "; ", index, "++ {")
 		} else if field.SizeFrom != "" {
 			g.P("for ", index, " := 0; ", index, " < len(", name, "); ", index, "++ {")
 		}
-		g.P("var ", char, " ", fieldGoType(g, field))
-		g.P("if ", index, " < len(", name, ") { ", char, " = ", name, "[", index, "] }")
-		name = char
+		if field.Length == 0 || field.SizeFrom != "" {
+			char := fmt.Sprintf("v%d", lvl)
+			g.P("var ", char, " ", fieldGoType(g, field), "// ", field.GoName)
+			g.P("if ", index, " < len(", name, ") { ", char, " = ", name, "[", index, "] }")
+			name = char
+		} else {
+			name = fmt.Sprintf("%s[%s]", name, index)
+		}
 	}
 
 	switch {
 	case field.TypeEnum != nil:
-		encodeBaseType(g, field.TypeEnum.Type, name, 0, "")
+		encodeBaseType(g, field.TypeEnum.Type, fieldGoType(g, field), name, 0, "", false)
 	case field.TypeAlias != nil:
 		alias := field.TypeAlias
 		if typ := alias.TypeStruct; typ != nil {
 			encodeFields(g, typ.Fields, name, lvl+1)
 		} else {
-			encodeBaseType(g, alias.Type, name, alias.Length, "")
+			if alias.Length > 0 {
+				encodeBaseType(g, alias.Type, BaseTypesGo[alias.Type], name, alias.Length, "", false)
+			} else {
+				encodeBaseType(g, alias.Type, fieldGoType(g, field), name, 0, "", false)
+			}
 		}
 	case field.TypeStruct != nil:
 		encodeFields(g, field.TypeStruct.Fields, name, lvl+1)
 	case field.TypeUnion != nil:
-		g.P("buf.EncodeBytes(", name, ".", fieldUnionData, "[:], 0)")
+		maxSize := getUnionSize(field.TypeUnion)
+		g.P("buf.EncodeBytes(", name, ".", fieldUnionData, "[:], ", maxSize, ")")
 	default:
 		logrus.Panicf("\t// ??? buf[pos] = %s (%s)\n", name, field.Type)
 	}
@@ -234,61 +219,52 @@ func encodeField(g *GenFile, field *Field, name string, getFieldName func(name s
 	}
 }
 
-func generateMessageMarshal(g *GenFile, name string, fields []*Field) {
-	g.P("func (m *", name, ") Marshal(b []byte) ([]byte, error) {")
-	g.P("var buf *", govppCodecPkg.Ident("Buffer"))
-	g.P("if b == nil {")
-	g.P("buf = ", govppCodecPkg.Ident("NewBuffer"), "(make([]byte, m.Size()))")
-	g.P("} else {")
-	g.P("buf = ", govppCodecPkg.Ident("NewBuffer"), "(b)")
-	g.P("}")
-
-	encodeFields(g, fields, "m", 0)
-
-	g.P("return buf.Bytes(), nil")
-	g.P("}")
-}
-
-func decodeBaseType(g *GenFile, typ, orig, name string, length int, sizefrom string, alloc bool) {
+func encodeBaseType(g *GenFile, typ, orig, name string, length int, sizefrom string, alloc bool) {
 	isArray := length > 0 || sizefrom != ""
 	if isArray {
 		switch typ {
 		case U8:
-			g.P("copy(", name, "[:], buf.DecodeBytes(", length, "))")
+			if alloc {
+				g.P("buf.EncodeBytes(", name, ", ", length, ")")
+			} else {
+				g.P("buf.EncodeBytes(", name, "[:], ", length, ")")
+			}
 			return
 		case I8, I16, U16, I32, U32, I64, U64, F64:
+			gotype := BaseTypesGo[typ]
+			if length != 0 {
+				g.P("for i := 0; i < ", length, "; i++ {")
+			} else if sizefrom != "" {
+				g.P("for i := 0; i < len(", name, "); i++ {")
+			}
 			if alloc {
-				var size string
-				switch {
-				case length > 0:
-					size = strconv.Itoa(length)
-				case sizefrom != "":
-					size = sizefrom
-				}
-				if size != "" {
-					g.P(name, " = make([]", orig, ", ", size, ")")
-				}
+				g.P("var x ", gotype)
+				g.P("if i < len(", name, ") { x = ", gotype, "(", name, "[i]) }")
+				name = "x"
 			}
-			g.P("for i := 0; i < len(", name, "); i++ {")
-			name = fmt.Sprintf("%s[i]", name)
 		}
 	}
+	conv := func(s string) string {
+		if gotype, ok := BaseTypesGo[typ]; !ok || gotype != orig {
+			return fmt.Sprintf("%s(%s)", gotype, s)
+		}
+		return s
+	}
 	switch typ {
-	case I8, U8, I16, U16, I32, U32, I64, U64:
+	case I8, I16, I32, I64:
 		typsize := BaseTypeSizes[typ]
-		if gotype, ok := BaseTypesGo[typ]; !ok || gotype != orig || strings.HasPrefix(orig, "i") {
-			g.P(name, " = ", orig, "(buf.DecodeUint", typsize*8, "())")
-		} else {
-			g.P(name, " = buf.DecodeUint", typsize*8, "()")
-		}
+		g.P("buf.EncodeInt", typsize*8, "(", conv(name), ")")
+	case U8, U16, U32, U64:
+		typsize := BaseTypeSizes[typ]
+		g.P("buf.EncodeUint", typsize*8, "(", conv(name), ")")
 	case F64:
-		g.P(name, " = ", orig, "(buf.DecodeFloat64())")
+		g.P("buf.EncodeFloat64(", conv(name), ")")
 	case BOOL:
-		g.P(name, " = buf.DecodeBool()")
+		g.P("buf.EncodeBool(", name, ")")
 	case STRING:
-		g.P(name, " = buf.DecodeString(", length, ")")
+		g.P("buf.EncodeString(", name, ", ", length, ")")
 	default:
-		logrus.Panicf("\t// ??? %s %s\n", name, typ)
+		logrus.Panicf("// ??? %s %s\n", name, typ)
 	}
 	if isArray {
 		switch typ {
@@ -298,7 +274,7 @@ func decodeBaseType(g *GenFile, typ, orig, name string, length int, sizefrom str
 	}
 }
 
-func generateMessageUnmarshal(g *GenFile, name string, fields []*Field) {
+func genMessageUnmarshal(g *GenFile, name string, fields []*Field) {
 	g.P("func (m *", name, ") Unmarshal(b []byte) error {")
 
 	if len(fields) > 0 {
@@ -338,7 +314,7 @@ func decodeField(g *GenFile, field *Field, name string, getFieldName func(string
 		if field.Length > 0 {
 			g.P("for ", index, " := 0; ", index, " < ", field.Length, ";", index, "++ {")
 		} else if field.SizeFrom != "" {
-			g.P(name, " = make(", getFieldType(g, field), ", int(", sizeFromName, "))")
+			g.P(name, " = make(", getFieldType(g, field), ", ", sizeFromName, ")")
 			g.P("for ", index, " := 0; ", index, " < len(", name, ");", index, "++ {")
 		}
 		name = fmt.Sprintf("%s[%s]", name, index)
@@ -357,7 +333,7 @@ func decodeField(g *GenFile, field *Field, name string, getFieldName func(string
 			if alias.Length > 0 {
 				decodeBaseType(g, alias.Type, BaseTypesGo[alias.Type], name, alias.Length, "", false)
 			} else {
-				decodeBaseType(g, alias.Type, fieldGoType(g, field), name, alias.Length, "", false)
+				decodeBaseType(g, alias.Type, fieldGoType(g, field), name, 0, "", false)
 			}
 		}
 	} else if typ := field.TypeStruct; typ != nil {
@@ -373,3 +349,60 @@ func decodeField(g *GenFile, field *Field, name string, getFieldName func(string
 		g.P("}")
 	}
 }
+
+func decodeBaseType(g *GenFile, typ, orig, name string, length int, sizefrom string, alloc bool) {
+	isArray := length > 0 || sizefrom != ""
+	if isArray {
+		var size string
+		switch {
+		case length > 0:
+			size = strconv.Itoa(length)
+		case sizefrom != "":
+			size = sizefrom
+		}
+		switch typ {
+		case U8:
+			if alloc {
+				g.P(name, " = make([]byte, ", size, ")")
+				g.P("copy(", name, ", buf.DecodeBytes(len(", name, ")))")
+			} else {
+				g.P("copy(", name, "[:], buf.DecodeBytes(", size, "))")
+			}
+			return
+		case I8, I16, U16, I32, U32, I64, U64, F64:
+			if alloc {
+				g.P(name, " = make([]", orig, ", ", size, ")")
+			}
+			g.P("for i := 0; i < len(", name, "); i++ {")
+			name = fmt.Sprintf("%s[i]", name)
+		}
+	}
+	conv := func(s string) string {
+		if gotype, ok := BaseTypesGo[typ]; !ok || gotype != orig {
+			return fmt.Sprintf("%s(%s)", orig, s)
+		}
+		return s
+	}
+	switch typ {
+	case I8, I16, I32, I64:
+		typsize := BaseTypeSizes[typ]
+		g.P(name, " = ", conv(fmt.Sprintf("buf.DecodeInt%d()", typsize*8)))
+	case U8, U16, U32, U64:
+		typsize := BaseTypeSizes[typ]
+		g.P(name, " = ", conv(fmt.Sprintf("buf.DecodeUint%d()", typsize*8)))
+	case F64:
+		g.P(name, " = ", conv("buf.DecodeFloat64()"))
+	case BOOL:
+		g.P(name, " = buf.DecodeBool()")
+	case STRING:
+		g.P(name, " = buf.DecodeString(", length, ")")
+	default:
+		logrus.Panicf("\t// ??? %s %s\n", name, typ)
+	}
+	if isArray {
+		switch typ {
+		case I8, U8, I16, U16, I32, U32, I64, U64, F64:
+			g.P("}")
+		}
+	}
+}
diff --git a/binapigen/gen_helpers.go b/binapigen/gen_helpers.go
index a22f1c6..5eafc76 100644
--- a/binapigen/gen_helpers.go
+++ b/binapigen/gen_helpers.go
@@ -25,7 +25,7 @@ const (
 	stringsPkg = GoImportPath("strings")
 )
 
-func generateIPConversion(g *GenFile, structName string, ipv int) {
+func genIPConversion(g *GenFile, structName string, ipv int) {
 	// ParseIPXAddress method
 	g.P("func Parse", structName, "(s string) (", structName, ", error) {")
 	if ipv == 4 {
@@ -77,7 +77,7 @@ func generateIPConversion(g *GenFile, structName string, ipv int) {
 	g.P()
 }
 
-func generateAddressConversion(g *GenFile, structName string) {
+func genAddressConversion(g *GenFile, structName string) {
 	// ParseAddress method
 	g.P("func Parse", structName, "(s string) (", structName, ", error) {")
 	g.P("	ip := ", netPkg.Ident("ParseIP"), "(s)")
@@ -132,7 +132,7 @@ func generateAddressConversion(g *GenFile, structName string) {
 	g.P()
 }
 
-func generateIPPrefixConversion(g *GenFile, structName string, ipv int) {
+func genIPPrefixConversion(g *GenFile, structName string, ipv int) {
 	// ParsePrefix method
 	g.P("func Parse", structName, "(s string) (prefix ", structName, ", err error) {")
 	g.P("	hasPrefix := ", stringsPkg.Ident("Contains"), "(s, \"/\")")
@@ -211,7 +211,7 @@ func generateIPPrefixConversion(g *GenFile, structName string, ipv int) {
 	g.P()
 }
 
-func generatePrefixConversion(g *GenFile, structName string) {
+func genPrefixConversion(g *GenFile, structName string) {
 	// ParsePrefix method
 	g.P("func Parse", structName, "(ip string) (prefix ", structName, ", err error) {")
 	g.P("	hasPrefix := ", stringsPkg.Ident("Contains"), "(ip, \"/\")")
@@ -276,7 +276,7 @@ func generatePrefixConversion(g *GenFile, structName string) {
 	g.P()
 }
 
-func generateAddressWithPrefixConversion(g *GenFile, structName string) {
+func genAddressWithPrefixConversion(g *GenFile, structName string) {
 	// ParseAddressWithPrefix method
 	g.P("func Parse", structName, "(s string) (", structName, ", error) {")
 	g.P("	prefix, err := ParsePrefix(s)")
@@ -308,7 +308,7 @@ func generateAddressWithPrefixConversion(g *GenFile, structName string) {
 	g.P()
 }
 
-func generateMacAddressConversion(g *GenFile, structName string) {
+func genMacAddressConversion(g *GenFile, structName string) {
 	// ParseMAC method
 	g.P("func Parse", structName, "(s string) (", structName, ", error) {")
 	g.P("	var macaddr ", structName)
diff --git a/binapigen/generate.go b/binapigen/generate.go
index 689463e..679dd54 100644
--- a/binapigen/generate.go
+++ b/binapigen/generate.go
@@ -237,13 +237,13 @@ func genAlias(g *GenFile, alias *Alias) {
 	// generate alias-specific methods
 	switch alias.Name {
 	case "ip4_address":
-		generateIPConversion(g, alias.GoName, 4)
+		genIPConversion(g, alias.GoName, 4)
 	case "ip6_address":
-		generateIPConversion(g, alias.GoName, 16)
+		genIPConversion(g, alias.GoName, 16)
 	case "address_with_prefix":
-		generateAddressWithPrefixConversion(g, alias.GoName)
+		genAddressWithPrefixConversion(g, alias.GoName)
 	case "mac_address":
-		generateMacAddressConversion(g, alias.GoName)
+		genMacAddressConversion(g, alias.GoName)
 	}
 }
 
@@ -257,7 +257,7 @@ func genStruct(g *GenFile, typ *Struct) {
 	} else {
 		g.P("type ", typ.GoName, " struct {")
 		for i := range typ.Fields {
-			generateField(g, typ.Fields, i)
+			genField(g, typ.Fields, i)
 		}
 		g.P("}")
 	}
@@ -266,13 +266,13 @@ func genStruct(g *GenFile, typ *Struct) {
 	// generate type-specific methods
 	switch typ.Name {
 	case "address":
-		generateAddressConversion(g, typ.GoName)
+		genAddressConversion(g, typ.GoName)
 	case "prefix":
-		generatePrefixConversion(g, typ.GoName)
+		genPrefixConversion(g, typ.GoName)
 	case "ip4_prefix":
-		generateIPPrefixConversion(g, typ.GoName, 4)
+		genIPPrefixConversion(g, typ.GoName, 4)
 	case "ip6_prefix":
-		generateIPPrefixConversion(g, typ.GoName, 6)
+		genIPPrefixConversion(g, typ.GoName, 6)
 	}
 }
 
@@ -313,7 +313,7 @@ func genUnionFieldMethods(g *GenFile, structName string, field *Field) {
 
 	// Setter
 	g.P("func (u *", structName, ") Set", field.GoName, "(a ", getterStruct, ") {")
-	g.P("	var buf = ", govppCodecPkg.Ident("NewBuffer"), "(u.", fieldUnionData, "[:])")
+	g.P("	buf := ", govppCodecPkg.Ident("NewBuffer"), "(u.", fieldUnionData, "[:])")
 	encodeField(g, field, "a", func(name string) string {
 		return "a." + name
 	}, 0)
@@ -321,7 +321,7 @@ func genUnionFieldMethods(g *GenFile, structName string, field *Field) {
 
 	// Getter
 	g.P("func (u *", structName, ") Get", field.GoName, "() (a ", getterStruct, ") {")
-	g.P("	var buf = ", govppCodecPkg.Ident("NewBuffer"), "(u.", fieldUnionData, "[:])")
+	g.P("	buf := ", govppCodecPkg.Ident("NewBuffer"), "(u.", fieldUnionData, "[:])")
 	decodeField(g, field, "a", func(name string) string {
 		return "a." + name
 	}, 0)
@@ -330,28 +330,28 @@ func genUnionFieldMethods(g *GenFile, structName string, field *Field) {
 	g.P()
 }
 
-func generateField(g *GenFile, fields []*Field, i int) {
+func genField(g *GenFile, fields []*Field, i int) {
 	field := fields[i]
 
 	logf(" gen FIELD[%d] %s (%s) - type: %q (array: %v/%v)", i, field.GoName, field.Name, field.Type, field.Array, field.Length)
 
 	gotype := getFieldType(g, field)
 	tags := structTags{
-		"binapi": fieldTagJSON(field),
-		"json":   fieldTagBinapi(field),
+		"binapi": fieldTagBinapi(field),
+		"json":   fieldTagJson(field),
 	}
 
 	g.P(field.GoName, " ", gotype, tags)
 }
 
-func fieldTagBinapi(field *Field) string {
+func fieldTagJson(field *Field) string {
 	if field.FieldSizeOf != nil {
 		return "-"
 	}
 	return fmt.Sprintf("%s,omitempty", field.Name)
 }
 
-func fieldTagJSON(field *Field) string {
+func fieldTagBinapi(field *Field) string {
 	typ := fromApiType(field.Type)
 	if field.Array {
 		if field.Length > 0 {
@@ -370,18 +370,15 @@ func fieldTagJSON(field *Field) string {
 		tag = append(tag, fmt.Sprintf("limit=%s", limit))
 	}
 	if def, ok := field.Meta["default"]; ok && def != nil {
-		actual := fieldActualType(field)
-		if t, ok := BaseTypesGo[actual]; ok {
-			switch t {
-			case I8, I16, I32, I64:
-				def = int(def.(float64))
-			case U8, U16, U32, U64:
-				def = uint(def.(float64))
-			case F64:
-				def = def.(float64)
-			}
+		switch fieldActualType(field) {
+		case I8, I16, I32, I64:
+			def = int(def.(float64))
+		case U8, U16, U32, U64:
+			def = uint(def.(float64))
+		case F64:
+			def = def.(float64)
 		}
-		tag = append(tag, fmt.Sprintf("default=%s", def))
+		tag = append(tag, fmt.Sprintf("default=%v", def))
 	}
 	return strings.Join(tag, ",")
 }
@@ -448,23 +445,23 @@ func genMessage(g *GenFile, msg *Message) {
 	} else {
 		g.P("type ", msg.GoIdent, " struct {")
 		for i := range msg.Fields {
-			generateField(g, msg.Fields, i)
+			genField(g, msg.Fields, i)
 		}
 		g.P("}")
 	}
 	g.P()
 
-	generateMessageMethods(g, msg)
+	genMessageMethods(g, msg)
 
 	// encoding methods
-	generateMessageSize(g, msg.GoIdent.GoName, msg.Fields)
-	generateMessageMarshal(g, msg.GoIdent.GoName, msg.Fields)
-	generateMessageUnmarshal(g, msg.GoIdent.GoName, msg.Fields)
+	genMessageSize(g, msg.GoIdent.GoName, msg.Fields)
+	genMessageMarshal(g, msg.GoIdent.GoName, msg.Fields)
+	genMessageUnmarshal(g, msg.GoIdent.GoName, msg.Fields)
 
 	g.P()
 }
 
-func generateMessageMethods(g *GenFile, msg *Message) {
+func genMessageMethods(g *GenFile, msg *Message) {
 	// Reset method
 	g.P("func (m *", msg.GoIdent.GoName, ") Reset() { *m = ", msg.GoIdent.GoName, "{} }")
 
@@ -481,3 +478,16 @@ func generateMessageMethods(g *GenFile, msg *Message) {
 
 	g.P()
 }
+
+func apiMsgType(t msgType) GoIdent {
+	switch t {
+	case msgTypeRequest:
+		return govppApiPkg.Ident("RequestMessage")
+	case msgTypeReply:
+		return govppApiPkg.Ident("ReplyMessage")
+	case msgTypeEvent:
+		return govppApiPkg.Ident("EventMessage")
+	default:
+		return govppApiPkg.Ident("OtherMessage")
+	}
+}
diff --git a/binapigen/generator.go b/binapigen/generator.go
index e42e7fb..ce0954a 100644
--- a/binapigen/generator.go
+++ b/binapigen/generator.go
@@ -19,7 +19,6 @@ import (
 	"bytes"
 	"fmt"
 	"go/ast"
-	"go/format"
 	"go/parser"
 	"go/printer"
 	"go/token"
@@ -86,12 +85,11 @@ func New(opts Options, apifiles []*vppapi.File, filesToGen []string) (*Generator
 	logrus.Debugf("adding %d VPP API files to generator", len(gen.apifiles))
 
 	for _, apifile := range gen.apifiles {
-		filename := getFilename(apifile)
-
 		if _, ok := gen.FilesByName[apifile.Name]; ok {
 			return nil, fmt.Errorf("duplicate file: %q", apifile.Name)
 		}
 
+		filename := getFilename(apifile)
 		file, err := newFile(gen, apifile, packageNames[filename], importPaths[filename])
 		if err != nil {
 			return nil, fmt.Errorf("loading file %s failed: %w", apifile.Name, err)
@@ -108,7 +106,7 @@ func New(opts Options, apifiles []*vppapi.File, filesToGen []string) (*Generator
 		for _, genfile := range gen.filesToGen {
 			file, ok := gen.FilesByName[genfile]
 			if !ok {
-				return nil, fmt.Errorf("no API file found for: %v", genfile)
+				return nil, fmt.Errorf("nol API file found for: %v", genfile)
 			}
 			file.Generate = true
 			// generate all imported files
@@ -162,6 +160,7 @@ type GenFile struct {
 	packageNames  map[GoImportPath]GoPackageName
 }
 
+// NewGenFile creates new generated file with
 func (g *Generator) NewGenFile(filename string, importPath GoImportPath) *GenFile {
 	f := &GenFile{
 		gen:           g,
@@ -213,6 +212,7 @@ func (g *GenFile) Content() ([]byte, error) {
 	return g.injectImports(g.buf.Bytes())
 }
 
+// injectImports parses source, injects import block declaration with all imports and return formatted
 func (g *GenFile) injectImports(original []byte) ([]byte, error) {
 	// Parse source code
 	fset := token.NewFileSet()
@@ -290,66 +290,6 @@ func (g *GenFile) injectImports(original []byte) ([]byte, error) {
 	return out.Bytes(), nil
 }
 
-// GoIdent is a Go identifier, consisting of a name and import path.
-// The name is a single identifier and may not be a dot-qualified selector.
-type GoIdent struct {
-	GoName       string
-	GoImportPath GoImportPath
-}
-
-func (id GoIdent) String() string {
-	return fmt.Sprintf("%q.%v", id.GoImportPath, id.GoName)
-}
-
-func newGoIdent(f *File, fullName string) GoIdent {
-	name := strings.TrimPrefix(fullName, string(f.PackageName)+".")
-	return GoIdent{
-		GoName:       camelCaseName(name),
-		GoImportPath: f.GoImportPath,
-	}
-}
-
-// GoImportPath is a Go import path for a package.
-type GoImportPath string
-
-func (p GoImportPath) String() string {
-	return strconv.Quote(string(p))
-}
-
-func (p GoImportPath) Ident(s string) GoIdent {
-	return GoIdent{GoName: s, GoImportPath: p}
-}
-
-type GoPackageName string
-
-func cleanPackageName(name string) GoPackageName {
-	return GoPackageName(sanitizedName(name))
-}
-
-func sanitizedName(name string) string {
-	switch name {
-	case "interface":
-		return "interfaces"
-	case "map":
-		return "maps"
-	default:
-		return name
-	}
-}
-
-// baseName returns the last path element of the name, with the last dotted suffix removed.
-func baseName(name string) string {
-	// First, find the last element
-	if i := strings.LastIndex(name, "/"); i >= 0 {
-		name = name[i+1:]
-	}
-	// Now drop the suffix
-	if i := strings.LastIndex(name, "."); i >= 0 {
-		name = name[:i]
-	}
-	return name
-}
-
 func writeSourceTo(outputFile string, b []byte) error {
 	// create output directory
 	packageDir := filepath.Dir(outputFile)
@@ -357,20 +297,13 @@ func writeSourceTo(outputFile string, b []byte) error {
 		return fmt.Errorf("creating output dir %s failed: %v", packageDir, err)
 	}
 
-	// format generated source code
-	gosrc, err := format.Source(b)
-	if err != nil {
-		_ = ioutil.WriteFile(outputFile, b, 0666)
-		return fmt.Errorf("formatting source code failed: %v", err)
-	}
-
 	// write generated code to output file
-	if err := ioutil.WriteFile(outputFile, gosrc, 0666); err != nil {
+	if err := ioutil.WriteFile(outputFile, b, 0666); err != nil {
 		return fmt.Errorf("writing to output file %s failed: %v", outputFile, err)
 	}
 
-	lines := bytes.Count(gosrc, []byte("\n"))
-	logf("wrote %d lines (%d bytes) to: %q", lines, len(gosrc), outputFile)
+	lines := bytes.Count(b, []byte("\n"))
+	logf("wrote %d lines (%d bytes) to: %q", lines, len(b), outputFile)
 
 	return nil
 }
diff --git a/binapigen/generator_test.go b/binapigen/generator_test.go
index aa4ee04..1dfaca4 100644
--- a/binapigen/generator_test.go
+++ b/binapigen/generator_test.go
@@ -21,7 +21,10 @@ import (
 func TestGoModule(t *testing.T) {
 	const expected = "git.fd.io/govpp.git/binapi"
 
-	impPath := resolveImportPath("../binapi")
+	impPath, err := resolveImportPath("../binapi")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
 	if impPath != expected {
 		t.Fatalf("expected: %q, got: %q", expected, impPath)
 	}
diff --git a/binapigen/run.go b/binapigen/run.go
index 88e32b7..d3a181a 100644
--- a/binapigen/run.go
+++ b/binapigen/run.go
@@ -48,8 +48,11 @@ func run(apiDir string, filesToGenerate []string, opts Options, fn func(*Generat
 	}
 
 	if opts.ImportPrefix == "" {
-		opts.ImportPrefix = resolveImportPath(opts.OutputDir)
-		logrus.Debugf("resolved import prefix: %s", opts.ImportPrefix)
+		opts.ImportPrefix, err = resolveImportPath(opts.OutputDir)
+		if err != nil {
+			return fmt.Errorf("cannot resolve import path for output dir %s: %w", opts.OutputDir, err)
+		}
+		logrus.Infof("resolved import path prefix: %s", opts.ImportPrefix)
 	}
 
 	gen, err := New(opts, apifiles, filesToGenerate)
@@ -93,10 +96,6 @@ func init() {
 	if debug := os.Getenv("DEBUG_GOVPP"); strings.Contains(debug, "binapigen") {
 		Logger.SetLevel(logrus.DebugLevel)
 		logrus.SetLevel(logrus.DebugLevel)
-	} else if debug != "" {
-		Logger.SetLevel(logrus.InfoLevel)
-	} else {
-		Logger.SetLevel(logrus.WarnLevel)
 	}
 }
 
@@ -104,32 +103,30 @@ func logf(f string, v ...interface{}) {
 	Logger.Debugf(f, v...)
 }
 
-func resolveImportPath(dir string) string {
+// resolveImportPath tries to resolve import path for a directory.
+func resolveImportPath(dir string) (string, error) {
 	absPath, err := filepath.Abs(dir)
 	if err != nil {
-		panic(err)
+		return "", err
 	}
 	modRoot := findGoModuleRoot(absPath)
 	if modRoot == "" {
-		logrus.Fatalf("module root not found at: %s", absPath)
+		return "", err
 	}
-	modPath := findModulePath(path.Join(modRoot, "go.mod"))
-	if modPath == "" {
-		logrus.Fatalf("module path not found")
+	modPath, err := readModulePath(path.Join(modRoot, "go.mod"))
+	if err != nil {
+		return "", err
 	}
 	relDir, err := filepath.Rel(modRoot, absPath)
 	if err != nil {
-		panic(err)
+		return "", err
 	}
-	return filepath.Join(modPath, relDir)
+	return filepath.Join(modPath, relDir), nil
 }
 
+// findGoModuleRoot looks for enclosing Go module.
 func findGoModuleRoot(dir string) (root string) {
-	if dir == "" {
-		panic("dir not set")
-	}
 	dir = filepath.Clean(dir)
-	// Look for enclosing go.mod.
 	for {
 		if fi, err := os.Stat(filepath.Join(dir, "go.mod")); err == nil && !fi.IsDir() {
 			return dir
@@ -143,18 +140,17 @@ func findGoModuleRoot(dir string) (root string) {
 	return ""
 }
 
-var (
-	modulePathRE = regexp.MustCompile(`module[ \t]+([^ \t\r\n]+)`)
-)
+var modulePathRE = regexp.MustCompile(`module[ \t]+([^ \t\r\n]+)`)
 
-func findModulePath(file string) string {
-	data, err := ioutil.ReadFile(file)
+// readModulePath reads module path from go.mod file.
+func readModulePath(gomod string) (string, error) {
+	data, err := ioutil.ReadFile(gomod)
 	if err != nil {
-		return ""
+		return "", err
 	}
 	m := modulePathRE.FindSubmatch(data)
 	if m == nil {
-		return ""
+		return "", err
 	}
-	return string(m[1])
+	return string(m[1]), nil
 }
diff --git a/binapigen/definitions.go b/binapigen/strings.go
index 3c8a874..1ab24e9 100644
--- a/binapigen/definitions.go
+++ b/binapigen/strings.go
@@ -15,8 +15,10 @@
 package binapigen
 
 import (
+	"go/token"
 	"strings"
 	"unicode"
+	"unicode/utf8"
 )
 
 // commonInitialisms is a set of common initialisms that need to stay in upper case.
@@ -74,7 +76,8 @@ var specialInitialisms = map[string]string{
 }
 
 func usesInitialism(s string) string {
-	if u := strings.ToUpper(s); commonInitialisms[u] {
+	u := strings.ToUpper(s)
+	if commonInitialisms[u] {
 		return u
 	} else if su, ok := specialInitialisms[u]; ok {
 		return su
@@ -151,3 +154,12 @@ func camelCaseName(name string) (should string) {
 	}
 	return string(runes)
 }
+
+// sanitizedName returns the string converted into a valid Go identifier.
+func sanitizedName(s string) string {
+	r, _ := utf8.DecodeRuneInString(s)
+	if token.Lookup(s).IsKeyword() || !unicode.IsLetter(r) {
+		return s + "s"
+	}
+	return s
+}
diff --git a/binapigen/definitions_test.go b/binapigen/strings_test.go
index 761c95f..761c95f 100644
--- a/binapigen/definitions_test.go
+++ b/binapigen/strings_test.go
diff --git a/binapigen/types.go b/binapigen/types.go
index 0a21622..1d0dae5 100644
--- a/binapigen/types.go
+++ b/binapigen/types.go
@@ -87,8 +87,10 @@ func fieldActualType(field *Field) (actual string) {
 		actual = field.TypeAlias.Type
 	case field.TypeEnum != nil:
 		actual = field.TypeEnum.Type
+	default:
+		actual = field.Type
 	}
-	return field.Type
+	return
 }
 
 func fieldGoType(g *GenFile, field *Field) string {