[Babel-users] Revised [PATCH 1/2] Allow to independently monitor events for neighbour, interface, route, xroute
Juliusz Chroboczek
jch at irif.fr
Mon Aug 12 14:08:58 BST 2019
Please don't use tabs.
> + if (strcmp(token2, "route") == 0)
Don't use space after the if.
> + else // monitor was detected but none of the specialties - monitoring everything.
> + c = -1;
> }
It's going to treat "monitor typo" as just "monitor". This doesn't make
sense to me.
> +#define CONFIG_ACTION_NEIGHBOUR_MONITOR 6
Wouldn't MONITOR_NEIGBOUR be more logical?
> + case CONFIG_ACTION_INTERFACE_MONITOR:
> + set_flag(&s->monitor, MONITOR_INTERFACE);
> + local_notify_all_interface_1(s);
> + break;
...
I suggest defining a separate function (say, monitor_mask) that maps the
constant to the flag. Perhaps just an array index. Then add a flags
value to local_notify_all_1 that specifies what you want to notify, and
say something like:
case CONFIG_ACTION_INTERFACE_MONITOR:
case CONFIG_ACTION_ROUTE_MONITOR:
case ...
mask = monitor_mask(rc);
s->monitor |= mask;
local_notify_all_1(s, mask);
break;
case CONFIG_ACTION_INTERFACE_UNMONITOR:
case CONFIG_ACTION_ROUTE_UNMONITOR:
s->monitof &= ~monitor_mask(rc);
break;
> - int monitor;
> + uint8_t monitor;
Please make this an unsigned int. We don't use stdint types in Babel (I'm
an old man).
-- Juliusz
More information about the Babel-users
mailing list