[Babel-users] [PATCH] v2: Allow to independently monitor events for neighbour, interface, route, xroute

[Christof Schulze]

>
>> +.B neighbour-monitor
>> +and
>> +.BR neighbour-unmonitor ;
>
>Please use the syntax "monitor neighbours" and "unmonitor neighbours".
>Two keywords.
Sure.

>
> > +#define CONFIG_ACTION_NEIGHBOUR_MONITOR 6
> > +#define CONFIG_ACTION_NEIGHBOUR_UNMONITOR 7
> > +#define CONFIG_ACTION_ROUTE_MONITOR 8
> > +#define CONFIG_ACTION_ROUTE_UNMONITOR 9
> > +#define CONFIG_ACTION_XROUTE_MONITOR 10
> > +#define CONFIG_ACTION_XROUTE_UNMONITOR 11
> > +#define CONFIG_ACTION_INTERFACE_MONITOR 12
> > +#define CONFIG_ACTION_INTERFACE_UNMONITOR 13
>
>Please use a single action with a parameter.
I am not sure what you mean by that. Even if the syntax is changed to a 
monitor action having a parameter, still a representation is needed 
somewhere.

>> +static void
>> +local_notify_all_1(struct local_socket *s)
>> +{
>> +    local_notify_all_interface_1(s);
>> +    local_notify_all_neighbour_1(s);
>> +    local_notify_all_xroute_1(s);
>> +    local_notify_all_route_1(s);
>>    }
>
>Why is that refactoring necessary?
While not strictly necessary the refactoring avoids duplicate code. 
local_notify_all_*_1() is called in from two places: once in 
local_notify_all_1() and once in local_read().

> > +inline void set_flag(uint8_t *d, uint8_t flag) {
> > + *d |= 0x01 << flag;
> > +}
>
>Please don't -- just but the bit manipulation inline, I find that 
>easier to read.
Really? I find set_flag(buffer, flag) much more readable than
*d |= 0x01 << flag or *d &= ~( 0x01 << flag ) It may just be a matter of 
practice though.

I guess I could also turn this into a macro...


Christof
-- 
()  ascii ribbon campaign - against html e-mail
/\  against proprietary attachments

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Allow-to-independently-monitor-events-for-neighbour-.patch
Type: text/x-diff
Size: 9435 bytes
Desc: not available
URL: <http://alioth-lists.debian.net/pipermail/babel-users/attachments/20190105/1e257d40/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <http://alioth-lists.debian.net/pipermail/babel-users/attachments/20190105/1e257d40/attachment.sig>