lvzixun / sproto-Csharp

A pure C# implementation of sproto.
140 stars 53 forks source link

fix SprotoStream expand bug #9

Closed m2q1n9 closed 8 years ago

m2q1n9 commented 8 years ago

size > 128 时expand会有问题

m2q1n9 commented 8 years ago

加了一个 在unity中使用的例子

lvzixun commented 8 years ago
  1. 我觉得 https://github.com/lvzixun/sproto-Csharp/blob/master/src/SprotoStream.cs#L105 这行里面的this._expand ();是没必要添加的。 set只能修改当前buffer内的元素。
  2. Merge remote-tracking branch 'upstream/master'这个提交现在在master上已经存在了。
m2q1n9 commented 8 years ago

SprotoTypeSerialize 里有直接set的

m2q1n9 commented 8 years ago

如果能保证 set只能修改当前buffer内的元素 确实不需要加 不过貌似不能保证

lvzixun commented 8 years ago

期望的应当是set只能修改当前buffer内的值。所有触发expand行为的,都应该只有一个接口write。如果出现了越界触发set,那应该是个bug。

lvzixun commented 8 years ago

你那边能否提供下回出现越界set的测试用例吗? 我刚在自己测试了下, 并未出现这样的情况。

lvzixun commented 8 years ago
  1. @m2q1n9 将SprotoStream的初始this.size改为1,之后去encode一个空的协议。将会在set_header_fn时出现异常。 主要原因是在调用Seek的时候并未expand。而且此时由于没有field需要encode,所以也不会调用write,同时也不不会触发expand。之后再进行close时需要回写两个字节长度的filed个数,但buffer长度是只有1, 所以就会抛出异常。 bugfix: d03fbd714797d0cab95c9a12be9a92671de9cc6c
  2. 之前你有说过遇到packing error的异常,应该是: https://github.com/cloudwu/sproto/pull/44 这个。 这个问题在云风lua版本的sproto也会出现。 bugfix: e3e3f5b69b2f4ea7a2ef6b86416efa610bfd8149
lvzixun commented 8 years ago

我更新了readme,添加了Use in Unity。 暂时先把这个PR给关闭吧。

m2q1n9 commented 8 years ago

好的~ 最近没遇到啥问题了