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

[Christof Schulze]

Hello Juliusz,

I didn't see any of these suggestions in the 1.9 branch. Is that on 
purpose?

Cheers

Christof

On Sat, Jan 05, 2019 at 12:03:28AM +0100, Christof Schulze wrote:

>>>+.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
/\  www.asciiribbon.org   - against proprietary attachments
-------------- 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/20190703/7bfb8724/attachment.sig>