Juliusz,

It seems like the patch attached to the mail I am replying got lost somehow. A few other things have changed as well, I'll rebase and send via separate Mail.


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

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Babel-users mailing list
[email protected]
https://alioth-lists.debian.net/cgi-bin/mailman/listinfo/babel-users

Reply via email to