On Sun, Apr 28, 2013 at 12:46:11AM +0800, Marek Lindner wrote:
On Saturday, April 27, 2013 23:59:53 Antonio Quartulli wrote:
--- a/gateway_common.c +++ b/gateway_common.c @@ -202,8 +202,8 @@ static void batadv_gw_tvlv_ogm_handler_v1(struct batadv_priv *bat_priv, gateway.bandwidth_up = 0; } else { gateway_ptr = tvlv_value;
gateway.bandwidth_down =ntohl(gateway_ptr->bandwidth_down); - gateway.bandwidth_up = ntohl(gateway_ptr->bandwidth_up); + gateway.bandwidth_down = gateway_ptr->bandwidth_down; + gateway.bandwidth_up = gateway_ptr->bandwidth_up; if ((gateway.bandwidth_down == 0) || (gateway.bandwidth_up == 0)) { gateway.bandwidth_down = 0;
There is no bug.
I did not intend to fix a bug, I just wanted to use the __be32 notation for stuff sent over the wire. Well, I have to admit I thought you forgot some htonl/ntohl (this is why the notation is useful, to spot missing conversions..) but this was not the case :)
The original code converted the incoming big endian value to host endian (whatever that is) and works with that value internally. Now that you reversed the logic you have to adjust *all* occurences. Especially, when the tvlv bandwidth value is copied into a gw_node struct. You can use sparse to help you find the sections in the code by changing gw_node to be32 as well.
So far everything seems to be converted the right way: sparse does not complain, and here is the code where the GW component does the assignment yu talked about:
420 gw_node->bandwidth_down = ntohl(gateway->bandwidth_down); 421 gw_node->bandwidth_up = ntohl(gateway->bandwidth_up);
However, I am not convinced that converting the values every single time we access the variable is the optimal approach.
True, not really beautiful but this will avoid future bugs thanks to the __be notation.
Cheers,