search

dubslow / toxcore

The future of online communications.
https://tox.im
GNU General Public License v3.0
0 stars 0 forks source link

Issues with new groupchats #1

Open dubslow opened 9 years ago

dubslow commented 9 years ago

@JFreegman @mannol @subliun

https://github.com/JFreegman/toxcore/compare/new_groupchats#diff-fc3b018cc395313cde80af80358a58cbR221 Typo that doesn't actually have any consequences

https://github.com/JFreegman/toxcore/compare/new_groupchats#diff-fc3b018cc395313cde80af80358a58cbR265 This and following lines are suspicious, couldn't get_close_nodes fall off the end of nodes, since it has no idea how long nodes is?

https://github.com/JFreegman/toxcore/compare/new_groupchats#diff-fc3b018cc395313cde80af80358a58cbR324 Seems like a logical error to me? Or at least a poorly named variable. Wait no, it's not a logical error, however it is most definitely a very poorly named variable. partial_update would be better, since update or update_only is as opposed to "new entry", which isn't clear upon first reading.

https://github.com/JFreegman/toxcore/compare/new_groupchats#diff-fc3b018cc395313cde80af80358a58cbR477 Comment doesn't appear to be substantiated

https://github.com/JFreegman/toxcore/compare/new_groupchats#diff-fc3b018cc395313cde80af80358a58cbR537 Shouldn't sigdata be longer than data?

https://github.com/JFreegman/toxcore/compare/new_groupchats#diff-fc3b018cc395313cde80af80358a58cbR554 Don't you need to account for endianness when copying structs? (Also I read something about padding/alignment as well?) Ditto https://github.com/JFreegman/toxcore/compare/new_groupchats#diff-fc3b018cc395313cde80af80358a58cbR662

^ Is what I found going through group_announce.c. I'm taking a break for now.

JFreegman commented 9 years ago
  1. No typo, I just felt like being fancy
  2. That's safe. get_close_nodes is limited by that define
  3. Yeah, bad name but not a mistake. Also that should return something.
  4. That occurs in dispatch_packet. Could make a note of that in the comment.
  5. No, both should be the same size. 6 & 7. Those structs are packed with __attribute__ ((__packed__)). I'm not 100% sure about the endianness, I'll look into it.
dubslow commented 9 years ago

@JFreegman Also https://github.com/JFreegman/toxcore/compare/new_groupchats#diff-ff594fb1e7513d30fa13afc4e3cb9103R212