search

protocol / beyond-bitswap

Other
34 stars 9 forks source link

some feedback from a hasty look #6

Closed raulk closed 3 years ago

raulk commented 3 years ago

https://github.com/adlrocha/beyond-bitswap/blob/050c87b18d014cd56f813ddeaa9dbfe9831b8f1a/testbed/utils/tcp.go#L129-L138

I would recommend simplifying this to a combination of LimitReader + io.Copy into ioutil.Discard... There may be some hidden bugs in this logic.

https://github.com/adlrocha/beyond-bitswap/blob/050c87b18d014cd56f813ddeaa9dbfe9831b8f1a/testbed/utils/tcp.go#L92-L107

This logic may have several bugs.

  1. It's possible that Read has only read N bytes (less than the length of the buffer), but you're writing the entire buffer, which means that you may be writing more bytes than you read (sending leftovers from previous reads that remain in the buffer).
  2. I'm not familiar with files.ToFile(), but at first sight it seems like on every iteration you're reading from the beginning of the file. You might want to seek to an offset, or better even, use io.Copy to just pipe through until EOF.
raulk commented 3 years ago

https://github.com/adlrocha/beyond-bitswap/blob/050c87b18d014cd56f813ddeaa9dbfe9831b8f1a/testbed/utils/tcp.go#L117-L119

I recommend you use the uvarints from the encoding/binary package, which are no longer evil.