On Wed, May 04, 2011 at 03:36:37PM +0200, Antonio Quartulli wrote:
On Wed, May 04, 2011 at 01:22:34PM +0200, Andrew Lunn wrote:
+struct roam_adv_packet {
- uint8_t packet_type;
- uint8_t version;
- uint8_t dst[6];
- uint8_t ttl;
- uint8_t src[6];
- uint8_t client[6];
+} __packed;
Maybe put ttl at the end, to help with alignment?
As I did for the tt_query packet, the initial four fields are the same as the unicast_packet so that I can exploit route_unicast_packet() instead of writing routing function.
Is that a major issue?
No. It just that gcc might optimize accesses to src and client as a word read + 1/2 word read, if they where 1/2 word aligned. With ttl where it is, src and client are in strange alignments, so gcc will have to do byte access. But this is not the fast path, so it does not matter much.
- tt_global_add(bat_priv, orig_node, roam_adv_packet->client,
atomic_read(&orig_node->last_ttvn) + 1, true);- /* Roaming phase starts: I have a new information but the ttvn has been
* incremented yet. This flag will make me check all the incoming* packets for the correct destination. */The grammar in that comment could be better:
/* Roaming phase starts: I have new information but the ttvn has not * been incremented yet. This flag will make me check all the incoming * packets for the correct destination. */
Thanks and sorry for my poor grammar :)
Actually, it is mostly very good....
Ok..I got the point. Maybe I will not be so drastic but I will follow your suggestion
Lots of small functions is my style. However, the Linux coding style documentation says something similar:
Chapter 6: Functions
Functions should be short and sweet, and do just one thing. They should fit on one or two screenfuls of text (the ISO/ANSI screen size is 80x24, as we all know), and do one thing and do that well.
It is well worth reading Documentation/CodingStyle
Andrew