From b98f526b8aecf4740f59ffb0dbc9f4b6a749a2d8 Mon Sep 17 00:00:00 2001 From: Shuhei Taunma Date: Tue, 10 Nov 2015 14:20:16 +0900 Subject: [PATCH 1/3] (C#) Add ByteBuffer property to Table --- net/FlatBuffers/Table.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/net/FlatBuffers/Table.cs b/net/FlatBuffers/Table.cs index 1a097d371..09b002850 100644 --- a/net/FlatBuffers/Table.cs +++ b/net/FlatBuffers/Table.cs @@ -27,6 +27,8 @@ namespace FlatBuffers protected int bb_pos; protected ByteBuffer bb; + public ByteBuffer ByteBuffer { get { return bb; } } + // Look up a field in the vtable, return an offset into the object, or 0 if the field is not // present. protected int __offset(int vtableOffset) From 2dfff15a9dfbe0d21d9803cb0753f9dd8ea35608 Mon Sep 17 00:00:00 2001 From: rw Date: Wed, 11 Nov 2015 17:08:16 -0800 Subject: [PATCH 2/3] Improve Builder user interface. + Add state to the Builder object to track if we are inside a table, and if we are finished building the buffer. + Use this data to check that a buffer is being built correctly. + Panic if a buffer is not being built correctly. + Test that the panics happen as expected. Based on d236dea13d2fdb9ad596679868eb4204c1562151. --- go/builder.go | 102 +++++++++++++++++++++++++++++++++-------------- tests/go_test.go | 30 +++++++------- 2 files changed, 88 insertions(+), 44 deletions(-) diff --git a/go/builder.go b/go/builder.go index 7533dddf2..cf21dd55d 100644 --- a/go/builder.go +++ b/go/builder.go @@ -6,14 +6,17 @@ package flatbuffers // A Builder constructs byte buffers in a last-first manner for simplicity and // performance. type Builder struct { + // `Bytes` gives raw access to the buffer. Most users will want to use + // FinishedBytes() instead. Bytes []byte - minalign int - vtable []UOffsetT - objectEnd UOffsetT - insideObject bool - vtables []UOffsetT - head UOffsetT + minalign int + vtable []UOffsetT + objectEnd UOffsetT + vtables []UOffsetT + head UOffsetT + nested bool + finished bool } // NewBuilder initializes a Builder of size `initial_size`. @@ -33,7 +36,7 @@ func NewBuilder(initialSize int) *Builder { } // Reset truncates the underlying Builder buffer, facilitating alloc-free -// reuse of a Builder. +// reuse of a Builder. It also resets bookkeeping data. func (b *Builder) Reset() { if b.Bytes != nil { b.Bytes = b.Bytes[:cap(b.Bytes)] @@ -49,12 +52,22 @@ func (b *Builder) Reset() { b.head = UOffsetT(len(b.Bytes)) b.minalign = 1 + b.nested = false + b.finished = false +} + +// FinishedBytes returns a pointer to the written data in the byte buffer. +// Panics if the builder is not in a finished state (which is caused by calling +// `Finish()`). +func (b *Builder) FinishedBytes() []byte { + b.assertFinished() + return b.Bytes[b.Head():] } // StartObject initializes bookkeeping for writing a new object. func (b *Builder) StartObject(numfields int) { - b.notNested() - b.insideObject = true + b.assertNotNested() + b.nested = true // use 32-bit offsets so that arithmetic doesn't overflow. if cap(b.vtable) < numfields || b.vtable == nil { @@ -129,7 +142,7 @@ func (b *Builder) WriteVtable() (n UOffsetT) { var off UOffsetT if b.vtable[i] != 0 { // Forward reference to field; - // use 32bit number to ensure no overflow: + // use 32bit number to assert no overflow: off = objectOffset - b.vtable[i] } @@ -173,11 +186,9 @@ func (b *Builder) WriteVtable() (n UOffsetT) { // EndObject writes data necessary to finish object construction. func (b *Builder) EndObject() UOffsetT { - if !b.insideObject { - panic("not in object") - } + b.assertNested() n := b.WriteVtable() - b.insideObject = false + b.nested = false return n } @@ -271,7 +282,8 @@ func (b *Builder) PrependUOffsetT(off UOffsetT) { // // +, where T is the type of elements of this vector. func (b *Builder) StartVector(elemSize, numElems, alignment int) UOffsetT { - b.notNested() + b.assertNotNested() + b.nested = true b.Prep(SizeUint32, elemSize*numElems) b.Prep(alignment, elemSize*numElems) // Just in case alignment > int. return b.Offset() @@ -279,14 +291,19 @@ func (b *Builder) StartVector(elemSize, numElems, alignment int) UOffsetT { // EndVector writes data necessary to finish vector construction. func (b *Builder) EndVector(vectorNumElems int) UOffsetT { + b.assertNested() + // we already made space for this, so write without PrependUint32 b.PlaceUOffsetT(UOffsetT(vectorNumElems)) + + b.nested = false return b.Offset() } // CreateString writes a null-terminated string as a vector. func (b *Builder) CreateString(s string) UOffsetT { - b.notNested() + b.assertNotNested() + b.nested = true b.Prep(int(SizeUOffsetT), (len(s)+1)*SizeByte) b.PlaceByte(0) @@ -301,7 +318,8 @@ func (b *Builder) CreateString(s string) UOffsetT { // CreateByteString writes a byte slice as a string (null-terminated). func (b *Builder) CreateByteString(s []byte) UOffsetT { - b.notNested() + b.assertNotNested() + b.nested = true b.Prep(int(SizeUOffsetT), (len(s)+1)*SizeByte) b.PlaceByte(0) @@ -316,6 +334,9 @@ func (b *Builder) CreateByteString(s []byte) UOffsetT { // CreateByteVector writes a ubyte vector func (b *Builder) CreateByteVector(v []byte) UOffsetT { + b.assertNotNested() + b.nested = true + b.Prep(int(SizeUOffsetT), len(v)*SizeByte) l := UOffsetT(len(v)) @@ -326,20 +347,38 @@ func (b *Builder) CreateByteVector(v []byte) UOffsetT { return b.EndVector(len(v)) } -func (b *Builder) notNested() { - // Check that no other objects are being built while making this - // object. If not, panic: - if b.insideObject { - panic("non-inline data write inside of object") +func (b *Builder) assertNested() { + // If you get this assert, you're in an object while trying to write + // data that belongs outside of an object. + // To fix this, write non-inline data (like vectors) before creating + // objects. + if !b.nested { + panic("Incorrect creation order: must be inside object.") } } -func (b *Builder) nested(obj UOffsetT) { - // Structs are always stored inline, so need to be created right - // where they are used. You'll get this panic if you created it - // elsewhere: - if obj != b.Offset() { - panic("inline data write outside of object") +func (b *Builder) assertNotNested() { + // If you hit this, you're trying to construct a Table/Vector/String + // during the construction of its parent table (between the MyTableBuilder + // and builder.Finish()). + // Move the creation of these sub-objects to above the MyTableBuilder to + // not get this assert. + // Ignoring this assert may appear to work in simple cases, but the reason + // it is here is that storing objects in-line may cause vtable offsets + // to not fit anymore. It also leads to vtable duplication. + if b.nested { + panic("Incorrect creation order: object must not be nested.") + } +} + +func (b *Builder) assertFinished() { + // If you get this assert, you're attempting to get access a buffer + // which hasn't been finished yet. Be sure to call builder.Finish() + // with your root table. + // If you really need to access an unfinished buffer, use the Bytes + // buffer directly. + if !b.finished { + panic("Incorrect use of FinishedBytes(): must call 'Finish' first.") } } @@ -483,7 +522,10 @@ func (b *Builder) PrependUOffsetTSlot(o int, x, d UOffsetT) { // In generated code, `d` is always 0. func (b *Builder) PrependStructSlot(voffset int, x, d UOffsetT) { if x != d { - b.nested(x) + b.assertNested() + if x != b.Offset() { + panic("inline data write outside of object") + } b.Slot(voffset) } } @@ -495,8 +537,10 @@ func (b *Builder) Slot(slotnum int) { // Finish finalizes a buffer, pointing to the given `rootTable`. func (b *Builder) Finish(rootTable UOffsetT) { + b.assertNotNested() b.Prep(b.minalign, SizeUOffsetT) b.PrependUOffsetT(rootTable) + b.finished = true } // vtableEqual compares an unwritten vtable to a written vtable. diff --git a/tests/go_test.go b/tests/go_test.go index 4eb8d3e6a..9d94277f7 100644 --- a/tests/go_test.go +++ b/tests/go_test.go @@ -70,10 +70,10 @@ func TestAll(t *testing.T) { // Verify that panics are raised during exceptional conditions: CheckNotInObjectError(t.Fatalf) - CheckObjectIsNestedError(t.Fatalf) CheckStringIsNestedError(t.Fatalf) CheckByteStringIsNestedError(t.Fatalf) CheckStructIsNotInlineError(t.Fatalf) + CheckFinishedBytesError(t.Fatalf) // Verify that using the generated Go code builds a buffer without // returning errors: @@ -1113,20 +1113,6 @@ func CheckNotInObjectError(fail func(string, ...interface{})) { b.EndObject() } -// CheckObjectIsNestedError verifies that an object can not be created inside -// another object. -func CheckObjectIsNestedError(fail func(string, ...interface{})) { - b := flatbuffers.NewBuilder(0) - b.StartObject(0) - defer func() { - r := recover() - if r == nil { - fail("expected panic in CheckObjectIsNestedError") - } - }() - b.StartObject(0) -} - // CheckStringIsNestedError verifies that a string can not be created inside // another object. func CheckStringIsNestedError(fail func(string, ...interface{})) { @@ -1169,6 +1155,20 @@ func CheckStructIsNotInlineError(fail func(string, ...interface{})) { b.PrependStructSlot(0, 1, 0) } +// CheckFinishedBytesError verifies that `FinishedBytes` panics if the table +// is not finished. +func CheckFinishedBytesError(fail func(string, ...interface{})) { + b := flatbuffers.NewBuilder(0) + + defer func() { + r := recover() + if r == nil { + fail("expected panic in CheckFinishedBytesError") + } + }() + b.FinishedBytes() +} + // CheckDocExample checks that the code given in FlatBuffers documentation // is syntactically correct. func CheckDocExample(buf []byte, off flatbuffers.UOffsetT, fail func(string, ...interface{})) { From 3232727ace325e8cb2732cf208e6d902a1160454 Mon Sep 17 00:00:00 2001 From: rw Date: Wed, 11 Nov 2015 19:43:53 -0800 Subject: [PATCH 3/3] Python: Improve Builder user interface. + Add state to the Builder object to track if we are inside a table, and if we are finished building the buffer. + Use this data to check that a buffer is being built correctly. + Raise an exception if a buffer is not being built correctly. + Test that the exceptions happen as expected. Based on d236dea. --- python/flatbuffers/builder.py | 56 +++++++++++++++++++++++------------ tests/py_test.py | 37 ++++++++++++----------- 2 files changed, 57 insertions(+), 36 deletions(-) diff --git a/python/flatbuffers/builder.py b/python/flatbuffers/builder.py index ed5097041..6e3465913 100644 --- a/python/flatbuffers/builder.py +++ b/python/flatbuffers/builder.py @@ -32,7 +32,7 @@ class OffsetArithmeticError(RuntimeError): pass -class NotInObjectError(RuntimeError): +class IsNotNestedError(RuntimeError): """ Error caused by using a Builder to write Object data when not inside an Object. @@ -40,7 +40,7 @@ class NotInObjectError(RuntimeError): pass -class ObjectIsNestedError(RuntimeError): +class IsNestedError(RuntimeError): """ Error caused by using a Builder to begin an Object when an Object is already being built. @@ -63,6 +63,12 @@ class BuilderSizeError(RuntimeError): """ pass +class BuilderNotFinishedError(RuntimeError): + """ + Error caused by not calling `Finish` before calling `Output`. + """ + pass + # VtableMetadataFields is the count of metadata fields in each vtable. VtableMetadataFields = 2 @@ -85,7 +91,7 @@ class Builder(object): """ __slots__ = ("Bytes", "current_vtable", "head", "minalign", "objectEnd", - "vtables") + "vtables", "nested", "finished") """ Maximum buffer size constant, in bytes. @@ -110,13 +116,19 @@ class Builder(object): self.minalign = 1 self.objectEnd = None self.vtables = [] + self.nested = False + self.finished = False def Output(self): """ Output returns the portion of the buffer that has been used for - writing data. + writing data. It raises BuilderNotFinishedError if the buffer has not + been finished with `Finish`. """ + if not self.finished: + raise BuilderNotFinishedError() + return self.Bytes[self.Head():] def StartObject(self, numfields): @@ -128,6 +140,7 @@ class Builder(object): self.current_vtable = [0 for _ in range_func(numfields)] self.objectEnd = self.Offset() self.minalign = 1 + self.nested = True def WriteVtable(self): """ @@ -236,10 +249,8 @@ class Builder(object): def EndObject(self): """EndObject writes data necessary to finish object construction.""" - if self.current_vtable is None: - msg = ("flatbuffers: Tried to write the end of an Object when " - "the Builder was not currently writing an Object.") - raise NotInObjectError(msg) + self.assertNested() + self.nested = False return self.WriteVtable() def growByteBuffer(self): @@ -336,6 +347,7 @@ class Builder(object): """ self.assertNotNested() + self.nested = True self.Prep(N.Uint32Flags.bytewidth, elemSize*numElems) self.Prep(alignment, elemSize*numElems) # In case alignment > int. return self.Offset() @@ -343,6 +355,8 @@ class Builder(object): def EndVector(self, vectorNumElems): """EndVector writes data necessary to finish vector construction.""" + self.assertNested() + self.nested = False # we already made space for this, so write without PrependUint32 self.PlaceUOffsetT(vectorNumElems) return self.Offset() @@ -351,6 +365,7 @@ class Builder(object): """CreateString writes a null-terminated byte string as a vector.""" self.assertNotNested() + self.nested = True if isinstance(s, compat.string_types): x = s.encode() @@ -369,18 +384,24 @@ class Builder(object): return self.EndVector(len(x)) + def assertNested(self): + """ + Check that we are in the process of building an object. + """ + + if not self.nested: + raise IsNotNestedError() + def assertNotNested(self): """ Check that no other objects are being built while making this object. If not, raise an exception. """ - if self.current_vtable is not None: - msg = ("flatbuffers: Tried to write a new Object when the " - "Builder was already writing an Object.") - raise ObjectIsNestedError(msg) + if self.nested: + raise IsNestedError() - def assertNested(self, obj): + def assertStructIsInline(self, obj): """ Structs are always stored inline, so need to be created right where they are used. You'll get this error if you created it @@ -399,11 +420,7 @@ class Builder(object): buffer. """ - if self.current_vtable is None: - msg = ("flatbuffers: Tried to write an Object field when " - "the Builder was not currently writing an Object.") - raise NotInObjectError(msg) - + self.assertNested() self.current_vtable[slotnum] = self.Offset() def Finish(self, rootTable): @@ -411,6 +428,7 @@ class Builder(object): N.enforce_number(rootTable, N.UOffsetTFlags) self.Prep(self.minalign, N.UOffsetTFlags.bytewidth) self.PrependUOffsetTRelative(rootTable) + self.finished = True return self.Head() def Prepend(self, flags, off): @@ -470,7 +488,7 @@ class Builder(object): N.enforce_number(d, N.UOffsetTFlags) if x != d: - self.assertNested(x) + self.assertStructIsInline(x) self.Slot(v) def PrependBool(self, x): self.Prepend(N.BoolFlags, x) diff --git a/tests/py_test.py b/tests/py_test.py index 4f3dae92e..cce317989 100644 --- a/tests/py_test.py +++ b/tests/py_test.py @@ -309,7 +309,7 @@ class TestByteLayout(unittest.TestCase): want_ints = list(map(integerize, want_chars_or_ints)) want = bytearray(want_ints) - got = builder.Output() + got = builder.Bytes[builder.Head():] # use the buffer directly self.assertEqual(want, got) def test_numbers(self): @@ -878,24 +878,23 @@ class TestAllCodePathsOfExampleSchema(unittest.TestCase): b = flatbuffers.Builder(0) # make a child Monster within a vector of Monsters: - MyGame.Example.Monster.MonsterStartTestarrayoftablesVector(b, 1) - MyGame.Example.Monster.MonsterStart(b) MyGame.Example.Monster.MonsterAddHp(b, 99) sub_monster = MyGame.Example.Monster.MonsterEnd(b) - b.Finish(sub_monster) - tables = b.EndVector(1) + # build the vector: + MyGame.Example.Monster.MonsterStartTestarrayoftablesVector(b, 1) + b.PrependUOffsetTRelative(sub_monster) + vec = b.EndVector(1) # make the parent monster and include the vector of Monster: MyGame.Example.Monster.MonsterStart(b) - MyGame.Example.Monster.MonsterAddTestarrayoftables(b, tables) + MyGame.Example.Monster.MonsterAddTestarrayoftables(b, vec) mon = MyGame.Example.Monster.MonsterEnd(b) b.Finish(mon) # inspect the resulting data: - mon2 = MyGame.Example.Monster.Monster.GetRootAsMonster(b.Bytes, - b.Head()) + mon2 = MyGame.Example.Monster.Monster.GetRootAsMonster(b.Output(), 0) self.assertEqual(99, mon2.Testarrayoftables(0).Hp()) self.assertEqual(1, mon2.TestarrayoftablesLength()) @@ -1050,7 +1049,7 @@ class TestVtableDeduplication(unittest.TestCase): b.PrependInt16Slot(3, 99, 0) obj2 = b.EndObject() - got = b.Output() + got = b.Bytes[b.Head():] want = bytearray([ 240, 255, 255, 255, # == -12. offset to dedupped vtable. @@ -1107,17 +1106,16 @@ class TestVtableDeduplication(unittest.TestCase): class TestExceptions(unittest.TestCase): - def test_not_in_object_error(self): - b = flatbuffers.Builder(0) - exc = None - assertRaises(self, lambda: b.EndObject(), - flatbuffers.builder.NotInObjectError) - def test_object_is_nested_error(self): b = flatbuffers.Builder(0) b.StartObject(0) assertRaises(self, lambda: b.StartObject(0), - flatbuffers.builder.ObjectIsNestedError) + flatbuffers.builder.IsNestedError) + + def test_object_is_not_nested_error(self): + b = flatbuffers.Builder(0) + assertRaises(self, lambda: b.EndObject(), + flatbuffers.builder.IsNotNestedError) def test_struct_is_not_inline_error(self): b = flatbuffers.Builder(0) @@ -1135,7 +1133,12 @@ class TestExceptions(unittest.TestCase): b.StartObject(0) s = 'test1' assertRaises(self, lambda: b.CreateString(s), - flatbuffers.builder.ObjectIsNestedError) + flatbuffers.builder.IsNestedError) + + def test_finished_bytes_error(self): + b = flatbuffers.Builder(0) + assertRaises(self, lambda: b.Output(), + flatbuffers.builder.BuilderNotFinishedError) def CheckAgainstGoldDataGo():