Fix Issue #21: automatic calculation of alignment padding after lists

11 files changed, 211 insertions, 52 deletions

diff --git a/nexgb/xgbgen/aligngap.go b/nexgb/xgbgen/aligngap.go
new file mode 100644
index 0000000..1e07e18
--- /dev/null
+++ b/nexgb/xgbgen/aligngap.go
@@ -0,0 +1,130 @@
+package main
+
+import (
+	"fmt"
+	"os"
+)
+
+func (p *Protocol) AddAlignGaps() {
+	for i := range p.Imports {
+		p.Imports[i].AddAlignGaps()
+	}
+	for i := range p.Types {
+		switch t := p.Types[i].(type) {
+		case *Struct:
+			t.Fields = addAlignGapsToFields(t.xmlName, t.Fields)
+		case *Event:
+			t.Fields = addAlignGapsToFields(t.xmlName, t.Fields)
+		case *Error:
+			t.Fields = addAlignGapsToFields(t.xmlName, t.Fields)
+		}
+	}
+	for i := range p.Requests {
+		p.Requests[i].Fields = addAlignGapsToFields(p.Requests[i].xmlName, p.Requests[i].Fields)
+		if p.Requests[i].Reply != nil {
+			p.Requests[i].Reply.Fields = addAlignGapsToFields(p.Requests[i].xmlName, p.Requests[i].Reply.Fields)
+		}
+	}
+}
+
+func addAlignGapsToFields(name string, fields []Field) []Field {
+	var i int
+	for i = 0; i < len(fields); i++ {
+		if _, ok := fields[i].(*ListField); ok {
+			break
+		}
+	}
+	if i >= len(fields) {
+		return fields
+	}
+
+	r := make([]Field, 0, len(fields)+2)
+	r = append(r, fields[:i]...)
+
+	r = append(r, fields[i])
+	for i = i + 1; i < len(fields); i++ {
+		switch f := fields[i].(type) {
+		case *ListField:
+			// ok, add padding
+			sz := xcbSizeOfType(f.Type)
+			switch {
+			case sz == 1:
+				// nothing
+			case sz == 2:
+				r = append(r, &PadField{0, 2})
+			case sz == 3:
+				panic(fmt.Errorf("Alignment is not a power of 2"))
+			case sz >= 4:
+				r = append(r, &PadField{0, 4})
+			}
+
+		case *LocalField:
+			// nothing
+
+		default:
+			fmt.Fprintf(os.Stderr, "Can't add alignment gaps, mix of list and non-list fields: %s\n", name)
+			return fields
+		}
+		r = append(r, fields[i])
+	}
+
+	return r
+}
+
+func xcbSizeOfField(fld Field) int {
+	switch f := fld.(type) {
+	case *PadField:
+		return int(f.Bytes)
+
+	case *SingleField:
+		return xcbSizeOfType(f.Type)
+
+	case *ListField:
+		return 0
+
+	case *ExprField:
+		return xcbSizeOfType(f.Type)
+
+	case *ValueField:
+		return xcbSizeOfType(f.MaskType)
+
+	case *SwitchField:
+		return 0
+
+	default:
+		return 0
+	}
+}
+
+func xcbSizeOfType(typ Type) int {
+	switch t := typ.(type) {
+	case *Resource:
+		return 4
+
+	case *TypeDef:
+		return t.Size().Eval()
+
+	case *Base:
+		return t.Size().Eval()
+
+	case *Struct:
+		sz := 0
+		for i := range t.Fields {
+			sz += xcbSizeOfField(t.Fields[i])
+		}
+		return sz
+
+	case *Union:
+		sz := 0
+		for i := range t.Fields {
+			csz := xcbSizeOfField(t.Fields[i])
+			if csz > sz {
+				sz = csz
+			}
+		}
+		return sz
+
+	default:
+		return 0
+	}
+}
diff --git a/nexgb/xgbgen/context.go b/nexgb/xgbgen/context.go
index af0f598..f64f339 100644
--- a/nexgb/xgbgen/context.go
+++ b/nexgb/xgbgen/context.go
@@ -50,6 +50,8 @@ func (c *Context) Morph(xmlBytes []byte) {
 	// Translate XML types to nice types
 	c.protocol = parsedXml.Translate(nil)
 
+	c.protocol.AddAlignGaps()
+
 	// Start with Go header.
 	c.Putln("// Package %s is the X client API for the %s extension.",
 		c.protocol.PkgName(), c.protocol.ExtXName)
diff --git a/nexgb/xgbgen/field.go b/nexgb/xgbgen/field.go
index 16760d4..bf3b3be 100644
--- a/nexgb/xgbgen/field.go
+++ b/nexgb/xgbgen/field.go
@@ -50,6 +50,7 @@ func (pad *PadField) Initialize(p *Protocol) {}
 // It is also used in size calculation.
 type PadField struct {
 	Bytes uint
+	Align uint16
 }
 
 func (p *PadField) SrcName() string {
@@ -65,7 +66,11 @@ func (f *PadField) SrcType() string {
 }
 
 func (p *PadField) Size() Size {
-	return newFixedSize(p.Bytes)
+	if p.Align > 0 {
+		return newFixedSize(uint(p.Align), false)
+	} else {
+		return newFixedSize(p.Bytes, true)
+	}
 }
 
 // SingleField represents most of the fields in an XML protocol description.
@@ -130,9 +135,9 @@ func (f *ListField) Length() Size {
 			Expr: &FieldRef{
 				Name: f.SrcName(),
 			},
-		})
+		}, true)
 	}
-	return newExpressionSize(f.LengthExpr)
+	return newExpressionSize(f.LengthExpr, true)
 }
 
 // Size computes the *size* of a list (in bytes).
@@ -142,8 +147,9 @@ func (f *ListField) Length() Size {
 // special function written in go_struct.go to compute the size (since the
 // size in this case can only be computed recursively).
 func (f *ListField) Size() Size {
+	elsz := f.Type.Size()
 	simpleLen := &Padding{
-		Expr: newBinaryOp("*", f.Length().Expression, f.Type.Size().Expression),
+		Expr: newBinaryOp("*", f.Length().Expression, elsz.Expression),
 	}
 
 	switch field := f.Type.(type) {
@@ -153,18 +159,18 @@ func (f *ListField) Size() Size {
 				Name: fmt.Sprintf("%sListSize", f.Type.SrcName()),
 				Expr: &FieldRef{Name: f.SrcName()},
 			}
-			return newExpressionSize(sizeFun)
+			return newExpressionSize(sizeFun, elsz.exact)
 		} else {
-			return newExpressionSize(simpleLen)
+			return newExpressionSize(simpleLen, elsz.exact)
 		}
 	case *Union:
-		return newExpressionSize(simpleLen)
+		return newExpressionSize(simpleLen, elsz.exact)
 	case *Base:
-		return newExpressionSize(simpleLen)
+		return newExpressionSize(simpleLen, elsz.exact)
 	case *Resource:
-		return newExpressionSize(simpleLen)
+		return newExpressionSize(simpleLen, elsz.exact)
 	case *TypeDef:
-		return newExpressionSize(simpleLen)
+		return newExpressionSize(simpleLen, elsz.exact)
 	default:
 		log.Panicf("Cannot compute list size with type '%T'.", f.Type)
 	}
@@ -258,7 +264,7 @@ func (f *ValueField) Size() Size {
 				},
 			},
 		},
-	})
+	}, true)
 	return maskSize.Add(listSize)
 }
 
@@ -270,7 +276,7 @@ func (f *ValueField) ListLength() Size {
 				Name: f.MaskName,
 			},
 		},
-	})
+	}, true)
 }
 
 func (f *ValueField) Initialize(p *Protocol) {
@@ -303,7 +309,7 @@ func (f *SwitchField) SrcType() string {
 // expression that finds *which* bitcase fields are included, and sums the
 // sizes of those fields.
 func (f *SwitchField) Size() Size {
-	return newFixedSize(0)
+	return newFixedSize(0, true)
 }
 
 func (f *SwitchField) Initialize(p *Protocol) {
diff --git a/nexgb/xgbgen/go.go b/nexgb/xgbgen/go.go
index 6c680e8..ace4e00 100644
--- a/nexgb/xgbgen/go.go
+++ b/nexgb/xgbgen/go.go
@@ -103,15 +103,27 @@ func (td *TypeDef) Define(c *Context) {
 
 // Pad fields
 func (f *PadField) Define(c *Context) {
-	c.Putln("// padding: %d bytes", f.Bytes)
+	if f.Align > 0 {
+		c.Putln("// alignment gap to multiple of %d", f.Align)
+	} else {
+		c.Putln("// padding: %d bytes", f.Bytes)
+	}
 }
 
 func (f *PadField) Read(c *Context, prefix string) {
-	c.Putln("b += %s // padding", f.Size())
+	if f.Align > 0 {
+		c.Putln("b = (b + %d) & ^%d // alignment gap", f.Align-1, f.Align-1)
+	} else {
+		c.Putln("b += %s // padding", f.Size())
+	}
 }
 
 func (f *PadField) Write(c *Context, prefix string) {
-	c.Putln("b += %s // padding", f.Size())
+	if f.Align > 0 {
+		c.Putln("b = (b + %d) & ^%d // alignment gap", f.Align-1, f.Align-1)
+	} else {
+		c.Putln("b += %s // padding", f.Size())
+	}
 }
 
 // Local fields
diff --git a/nexgb/xgbgen/go_list.go b/nexgb/xgbgen/go_list.go
index fa8df69..1e85d9f 100644
--- a/nexgb/xgbgen/go_list.go
+++ b/nexgb/xgbgen/go_list.go
@@ -21,7 +21,6 @@ func (f *ListField) Read(c *Context, prefix string) {
 		c.Putln("for i := 0; i < int(%s); i++ {", length)
 		ReadSimpleSingleField(c, fmt.Sprintf("%s%s[i]", prefix, f.SrcName()), t)
 		c.Putln("}")
-		c.Putln("b = xgb.Pad(b)")
 	case *Base:
 		length := f.LengthExpr.Reduce(prefix)
 		if strings.ToLower(t.XmlName()) == "char" {
@@ -38,7 +37,7 @@ func (f *ListField) Read(c *Context, prefix string) {
 			c.Putln("%s%s = make([]%s, %s)",
 				prefix, f.SrcName(), t.SrcName(), length)
 			c.Putln("copy(%s%s[:%s], buf[b:])", prefix, f.SrcName(), length)
-			c.Putln("b += xgb.Pad(int(%s))", length)
+			c.Putln("b += int(%s)", length)
 		} else {
 			c.Putln("%s%s = make([]%s, %s)",
 				prefix, f.SrcName(), t.SrcName(), length)
@@ -46,7 +45,6 @@ func (f *ListField) Read(c *Context, prefix string) {
 			ReadSimpleSingleField(c,
 				fmt.Sprintf("%s%s[i]", prefix, f.SrcName()), t)
 			c.Putln("}")
-			c.Putln("b = xgb.Pad(b)")
 		}
 	case *TypeDef:
 		length := f.LengthExpr.Reduce(prefix)
@@ -55,7 +53,6 @@ func (f *ListField) Read(c *Context, prefix string) {
 		c.Putln("for i := 0; i < int(%s); i++ {", length)
 		ReadSimpleSingleField(c, fmt.Sprintf("%s%s[i]", prefix, f.SrcName()), t)
 		c.Putln("}")
-		c.Putln("b = xgb.Pad(b)")
 	case *Union:
 		c.Putln("%s%s = make([]%s, %s)",
 			prefix, f.SrcName(), t.SrcName(), f.LengthExpr.Reduce(prefix))
@@ -80,18 +77,16 @@ func (f *ListField) Write(c *Context, prefix string) {
 		WriteSimpleSingleField(c,
 			fmt.Sprintf("%s%s[i]", prefix, f.SrcName()), t)
 		c.Putln("}")
-		c.Putln("b = xgb.Pad(b)")
 	case *Base:
 		length := f.Length().Reduce(prefix)
 		if t.SrcName() == "byte" {
 			c.Putln("copy(buf[b:], %s%s[:%s])", prefix, f.SrcName(), length)
-			c.Putln("b += xgb.Pad(int(%s))", length)
+			c.Putln("b += int(%s)", length)
 		} else {
 			c.Putln("for i := 0; i < int(%s); i++ {", length)
 			WriteSimpleSingleField(c,
 				fmt.Sprintf("%s%s[i]", prefix, f.SrcName()), t)
 			c.Putln("}")
-			c.Putln("b = xgb.Pad(b)")
 		}
 	case *TypeDef:
 		length := f.Length().Reduce(prefix)
@@ -99,7 +94,6 @@ func (f *ListField) Write(c *Context, prefix string) {
 		WriteSimpleSingleField(c,
 			fmt.Sprintf("%s%s[i]", prefix, f.SrcName()), t)
 		c.Putln("}")
-		c.Putln("b = xgb.Pad(b)")
 	case *Union:
 		c.Putln("b += %sListBytes(buf[b:], %s%s)",
 			t.SrcName(), prefix, f.SrcName())
diff --git a/nexgb/xgbgen/go_request_reply.go b/nexgb/xgbgen/go_request_reply.go
index a2c6d9c..d82e157 100644
--- a/nexgb/xgbgen/go_request_reply.go
+++ b/nexgb/xgbgen/go_request_reply.go
@@ -138,18 +138,32 @@ func (r *Request) ReadReply(c *Context) {
 }
 
 func (r *Request) WriteRequest(c *Context) {
-	writeSize := func() {
-		c.Putln("xgb.Put16(buf[b:], uint16(size / 4)) " +
-			"// write request size in 4-byte units")
+	sz := r.Size(c)
+	writeSize1 := func() {
+		if sz.exact {
+			c.Putln("xgb.Put16(buf[b:], uint16(size / 4)) " +
+				"// write request size in 4-byte units")
+		} else {
+			c.Putln("blen := b")
+		}
 		c.Putln("b += 2")
 		c.Putln("")
 	}
+	writeSize2 := func() {
+		if sz.exact {
+			c.Putln("return buf")
+			return
+		}
+		c.Putln("b = xgb.Pad(b)")
+		c.Putln("xgb.Put16(buf[blen:], uint16(b / 4)) // write request size in 4-byte units")
+		c.Putln("return buf[:b]")
+	}
 	c.Putln("// Write request to wire for %s", r.SrcName())
 	c.Putln("// %s writes a %s request to a byte slice.",
 		r.ReqName(), r.SrcName())
 	c.Putln("func %s(c *xgb.Conn, %s) []byte {",
 		r.ReqName(), r.ParamNameTypes())
-	c.Putln("size := %s", r.Size(c))
+	c.Putln("size := %s", sz)
 	c.Putln("b := 0")
 	c.Putln("buf := make([]byte, size)")
 	c.Putln("")
@@ -165,18 +179,18 @@ func (r *Request) WriteRequest(c *Context) {
 		if !c.protocol.isExt() {
 			c.Putln("b += 1 // padding")
 		}
-		writeSize()
+		writeSize1()
 	} else if c.protocol.isExt() {
-		writeSize()
+		writeSize1()
 	}
 	for i, field := range r.Fields {
 		field.Write(c, "")
 		c.Putln("")
 		if i == 0 && !c.protocol.isExt() {
-			writeSize()
+			writeSize1()
 		}
 	}
-	c.Putln("return buf")
+	writeSize2()
 	c.Putln("}")
 	c.Putln("")
 }
diff --git a/nexgb/xgbgen/go_struct.go b/nexgb/xgbgen/go_struct.go
index 0f18084..ee74d90 100644
--- a/nexgb/xgbgen/go_struct.go
+++ b/nexgb/xgbgen/go_struct.go
@@ -78,7 +78,7 @@ func (s *Struct) Write(c *Context) {
 		field.Write(c, "v.")
 		c.Putln("")
 	}
-	c.Putln("return buf")
+	c.Putln("return buf[:b]")
 	c.Putln("}")
 	c.Putln("")
 }
diff --git a/nexgb/xgbgen/request_reply.go b/nexgb/xgbgen/request_reply.go
index 11a4e44..ae4eccb 100644
--- a/nexgb/xgbgen/request_reply.go
+++ b/nexgb/xgbgen/request_reply.go
@@ -91,7 +91,7 @@ func (r *Request) CookieName() string {
 // If it's a core protocol request, then we only account for *three*
 // bytes of the header (remove the extension opcode).
 func (r *Request) Size(c *Context) Size {
-	size := newFixedSize(0)
+	size := newFixedSize(0, true)
 
 	// If this is a core protocol request, we squeeze in an extra byte of
 	// data (from the fields below) between the opcode and the size of the
@@ -99,9 +99,9 @@ func (r *Request) Size(c *Context) Size {
 	// by the opcode of the request (while the first byte is always occupied
 	// by the opcode of the extension).
 	if !c.protocol.isExt() {
-		size = size.Add(newFixedSize(3))
+		size = size.Add(newFixedSize(3, true))
 	} else {
-		size = size.Add(newFixedSize(4))
+		size = size.Add(newFixedSize(4, true))
 	}
 
 	for _, field := range r.Fields {
@@ -122,7 +122,7 @@ func (r *Request) Size(c *Context) Size {
 	}
 	return newExpressionSize(&Padding{
 		Expr: size.Expression,
-	})
+	}, size.exact)
 }
 
 // Reply encapsulates the fields associated with a 'reply' element.
@@ -136,10 +136,10 @@ type Reply struct {
 // 2 bytes: A sequence number
 // 4 bytes: Number of additional bytes in 4-byte units past initial 32 bytes.
 func (r *Reply) Size() Size {
-	size := newFixedSize(0)
+	size := newFixedSize(0, true)
 
 	// Account for reply discriminant, sequence number and reply length
-	size = size.Add(newFixedSize(7))
+	size = size.Add(newFixedSize(7, true))
 
 	for _, field := range r.Fields {
 		size = size.Add(field.Size())
diff --git a/nexgb/xgbgen/size.go b/nexgb/xgbgen/size.go
index 8836892..6e49371 100644
--- a/nexgb/xgbgen/size.go
+++ b/nexgb/xgbgen/size.go
@@ -7,24 +7,25 @@ package main
 // for adding and multiplying sizes.
 type Size struct {
 	Expression
+	exact bool
 }
 
 // newFixedSize creates a new Size with some fixed and known value.
-func newFixedSize(fixed uint) Size {
-	return Size{&Value{v: int(fixed)}}
+func newFixedSize(fixed uint, exact bool) Size {
+	return Size{&Value{v: int(fixed)}, exact}
 }
 
 // newExpressionSize creates a new Size with some expression.
-func newExpressionSize(variable Expression) Size {
-	return Size{variable}
+func newExpressionSize(variable Expression, exact bool) Size {
+	return Size{variable, exact}
 }
 
 // Add adds s1 and s2 and returns a new Size.
 func (s1 Size) Add(s2 Size) Size {
-	return Size{newBinaryOp("+", s1, s2)}
+	return Size{newBinaryOp("+", s1, s2), s1.exact && s2.exact}
 }
 
 // Multiply mupltiplies s1 and s2 and returns a new Size.
 func (s1 Size) Multiply(s2 Size) Size {
-	return Size{newBinaryOp("*", s1, s2)}
+	return Size{newBinaryOp("*", s1, s2), s1.exact && s2.exact}
 }
diff --git a/nexgb/xgbgen/translation.go b/nexgb/xgbgen/translation.go
index b7e67e2..f595e5f 100644
--- a/nexgb/xgbgen/translation.go
+++ b/nexgb/xgbgen/translation.go
@@ -43,7 +43,7 @@ func (xml *XML) Translate(parent *Protocol) *Protocol {
 		newBaseType := &Base{
 			srcName: srcName,
 			xmlName: xmlName,
-			size:    newFixedSize(BaseTypeSizes[xmlName]),
+			size:    newFixedSize(BaseTypeSizes[xmlName], true),
 		}
 		protocol.Types = append(protocol.Types, newBaseType)
 	}
diff --git a/nexgb/xgbgen/type.go b/nexgb/xgbgen/type.go
index ded5be2..59f1a2d 100644
--- a/nexgb/xgbgen/type.go
+++ b/nexgb/xgbgen/type.go
@@ -154,7 +154,7 @@ func (r *Resource) XmlName() string {
 }
 
 func (r *Resource) Size() Size {
-	return newFixedSize(BaseTypeSizes["Id"])
+	return newFixedSize(BaseTypeSizes["Id"], true)
 }
 
 func (r *Resource) Initialize(p *Protocol) {
@@ -201,7 +201,7 @@ func (e *Event) XmlName() string {
 }
 
 func (e *Event) Size() Size {
-	return newExpressionSize(&Value{v: 32})
+	return newExpressionSize(&Value{v: 32}, true)
 }
 
 func (e *Event) Initialize(p *Protocol) {
@@ -231,7 +231,7 @@ func (e *EventCopy) XmlName() string {
 }
 
 func (e *EventCopy) Size() Size {
-	return newExpressionSize(&Value{v: 32})
+	return newExpressionSize(&Value{v: 32}, true)
 }
 
 func (e *EventCopy) Initialize(p *Protocol) {
@@ -262,7 +262,7 @@ func (e *Error) XmlName() string {
 }
 
 func (e *Error) Size() Size {
-	return newExpressionSize(&Value{v: 32})
+	return newExpressionSize(&Value{v: 32}, true)
 }
 
 func (e *Error) Initialize(p *Protocol) {
@@ -296,7 +296,7 @@ func (e *ErrorCopy) XmlName() string {
 }
 
 func (e *ErrorCopy) Size() Size {
-	return newExpressionSize(&Value{v: 32})
+	return newExpressionSize(&Value{v: 32}, true)
 }
 
 func (e *ErrorCopy) Initialize(p *Protocol) {
@@ -330,7 +330,7 @@ func (s *Struct) XmlName() string {
 }
 
 func (s *Struct) Size() Size {
-	size := newFixedSize(0)
+	size := newFixedSize(0, true)
 	for _, field := range s.Fields {
 		size = size.Add(field.Size())
 	}