Hello Sebastian,
On 8 Jan 2017, at 22:10, Sebastian Benoit wrote:
Job Snijders(j...@instituut.net) on 2017.01.08 20:24:19 +0100:
Dear OpenBSD developers,
This patch adds support for the "BGP Administrative Shutdown
Communication" to bgpd(8) and bgpctl(8).
Hi Job and Peter,
thanks, this is nice!
Thank you :)
.Re
.Pp
.Rs
+.%A J. Snijders
+.%A J. Heitz
+.%A K. Patel
+.%A I. Bagdonas
+.%A A. Simpson
+.%D January 2017
+.%R draft-ietf-idr-large-community
+.%T Large BGP Communities Attribute
this duplicates this entry
also newer stuff sould go to the bottom of the list i believe.
While there is some overlap in authors, these are different entries. If
it should go to the bottom then both of them are in the wrong place -
maybe we should fix this afterwards in a separate commit?
+.%D January 2017
+.%R draft-ietf-idr-shutdown
+.%T BGP Administrative Shutdown Communication
+ p+=shutcomm_len;
please add spaces around +=
Fixed locally + next three.
+ p = communication;
+ for (q = buf; *p && q < &buf[sizeof(buf) - 1]; p++) {
+ if (*p == '\n')
+ *q++ = ' ';
+ else
+ q = vis(q, *p, 0, 0);
+ }
+ *q = '\0';
+
+ return buf;
while i think this is correct, would it not be easier to encode \n as
control char with VIS_NL?
then you could just use strnvis()
Ah! I just stole this code from syslogd.c without checking if we could
do better. Have changed it to a single strnvis call locally, with
VIS_NL, and also added VIS_OCTAL while at it because it will be more
familiar to admins and because the tcpdump patch for shutcomm also uses
octal.
Thanks for the review, Sebastian. I’ll wait for some more feedback
before resubmitting.
Kind regards,
--
Peter van Dijk
PowerDNS.COM BV - https://www.powerdns.com/
