On Fri, Aug 29, 2014 at 11:25:52PM +0200, Alexander Bluhm wrote: > With this diff all my regression tests for syslogd pass. I will > try to pull parts of the diff into separate changes to make review > easier. I have not tested the syslogc feature yet. So I will write > more tests before committing this.
The tests for syslog control sockets are commited. Next step to libevent in syslogd is to cleanup the control connection code. - Name variable path_ctlsock consistently. - Name function ctlconn_logto() consistently. - Replace the nested if/else logic in ctlconn_write_handler() with if/return. - Call ctlconn_cleanup() only if there is a control connection. ok? bluhm Index: usr.sbin/syslogd/privsep.c =================================================================== RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/privsep.c,v retrieving revision 1.45 diff -u -p -u -p -r1.45 privsep.c --- usr.sbin/syslogd/privsep.c 10 Sep 2014 13:16:20 -0000 1.45 +++ usr.sbin/syslogd/privsep.c 13 Sep 2014 23:48:26 -0000 @@ -374,8 +374,8 @@ priv_init(char *conf, int numeric, int l for (i = 0; i < nunix; i++) if (pfd[PFD_UNIX_0 + i].fd != -1) (void)unlink(path_unix[i]); - if (ctlsock_path != NULL && pfd[PFD_CTLSOCK].fd != -1) - (void)unlink(ctlsock_path); + if (path_ctlsock != NULL && pfd[PFD_CTLSOCK].fd != -1) + (void)unlink(path_ctlsock); if (restart) { int r; Index: usr.sbin/syslogd/syslogd.c =================================================================== RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.c,v retrieving revision 1.124 diff -u -p -u -p -r1.124 syslogd.c --- usr.sbin/syslogd/syslogd.c 10 Sep 2014 13:16:20 -0000 1.124 +++ usr.sbin/syslogd/syslogd.c 14 Sep 2014 00:12:24 -0000 @@ -199,7 +199,7 @@ int IPv4Only = 0; /* when true, disable int IPv6Only = 0; /* when true, disable IPv4 */ int IncludeHostname = 0; /* include RFC 3164 style hostnames when forwarding */ -char *ctlsock_path = NULL; /* Path to control socket */ +char *path_ctlsock = NULL; /* Path to control socket */ #define CTL_READING_CMD 1 #define CTL_WRITING_REPLY 2 @@ -284,7 +284,7 @@ void ctlsock_accept_handler(void); void ctlconn_read_handler(void); void ctlconn_write_handler(void); void tailify_replytext(char *, int); -void logto_ctlconn(char *); +void ctlconn_logto(char *); int main(int argc, char *argv[]) @@ -335,7 +335,7 @@ main(int argc, char *argv[]) path_unix[nunix++] = optarg; break; case 's': - ctlsock_path = optarg; + path_ctlsock = optarg; break; default: usage(); @@ -460,8 +460,8 @@ main(int argc, char *argv[]) pfd[PFD_SENDSYS].fd = fd; pfd[PFD_SENDSYS].events = POLLIN; - if (ctlsock_path != NULL) { - fd = unix_socket(ctlsock_path, SOCK_STREAM, 0600); + if (path_ctlsock != NULL) { + fd = unix_socket(path_ctlsock, SOCK_STREAM, 0600); if (fd != -1) { if (listen(fd, 16) == -1) { logerror("ctlsock listen"); @@ -1075,7 +1075,7 @@ fprintlog(struct filed *f, int flags, ch if (ringbuf_append_line(f->f_un.f_mb.f_rb, line) == 1) f->f_un.f_mb.f_overflow = 1; if (f->f_un.f_mb.f_attached) - logto_ctlconn(line); + ctlconn_logto(line); break; } f->f_prevcount = 0; @@ -1893,8 +1893,8 @@ ctlconn_cleanup(void) { struct filed *f; - if (pfd[PFD_CTLCONN].fd != -1) - close(pfd[PFD_CTLCONN].fd); + if (close(pfd[PFD_CTLCONN].fd) == -1) + logerror("close ctlconn"); pfd[PFD_CTLCONN].fd = -1; pfd[PFD_CTLCONN].events = pfd[PFD_CTLCONN].revents = 0; @@ -1923,7 +1923,8 @@ ctlsock_accept_handler(void) return; } - ctlconn_cleanup(); + if (pfd[PFD_CTLCONN].fd != -1) + ctlconn_cleanup(); /* Only one connection at a time */ pfd[PFD_CTLSOCK].events = pfd[PFD_CTLSOCK].revents = 0; @@ -1984,7 +1985,6 @@ ctlconn_read_handler(void) default: ctl_cmd_bytes += n; } - if (ctl_cmd_bytes < sizeof(ctl_cmd)) return; @@ -2086,7 +2086,6 @@ ctlconn_read_handler(void) /* another syslogc can kick us out */ if (ctl_state == CTL_WRITING_CONT_REPLY) pfd[PFD_CTLSOCK].events = POLLIN; - } void @@ -2101,6 +2100,7 @@ ctlconn_write_handler(void) ctlconn_cleanup(); return; } + retry: n = write(pfd[PFD_CTLCONN].fd, ctl_reply + ctl_reply_offset, ctl_reply_size - ctl_reply_offset); @@ -2117,26 +2117,29 @@ ctlconn_write_handler(void) default: ctl_reply_offset += n; } - if (ctl_reply_offset >= ctl_reply_size) { - /* - * Make space in the buffer for continous writes. - * Set offset behind reply header to skip it - */ - if (ctl_state == CTL_WRITING_CONT_REPLY) { - *reply_text = '\0'; - ctl_reply_offset = ctl_reply_size = CTL_REPLY_SIZE; - - /* Now is a good time to report dropped lines */ - if (membuf_drop) { - strlcat(reply_text, "<ENOBUFS>\n", MAX_MEMBUF); - ctl_reply_size = CTL_REPLY_SIZE; - membuf_drop = 0; - } else { - /* Nothing left to write */ - pfd[PFD_CTLCONN].events = POLLIN; - } - } else - ctlconn_cleanup(); + if (ctl_reply_offset < ctl_reply_size) + return; + + if (ctl_state != CTL_WRITING_CONT_REPLY) { + ctlconn_cleanup(); + return; + } + + /* + * Make space in the buffer for continous writes. + * Set offset behind reply header to skip it + */ + *reply_text = '\0'; + ctl_reply_offset = ctl_reply_size = CTL_REPLY_SIZE; + + /* Now is a good time to report dropped lines */ + if (membuf_drop) { + strlcat(reply_text, "<ENOBUFS>\n", MAX_MEMBUF); + ctl_reply_size = CTL_REPLY_SIZE; + membuf_drop = 0; + } else { + /* Nothing left to write */ + pfd[PFD_CTLCONN].events = POLLIN; } } @@ -2163,7 +2166,7 @@ tailify_replytext(char *replytext, int l } void -logto_ctlconn(char *line) +ctlconn_logto(char *line) { size_t l; Index: usr.sbin/syslogd/syslogd.h =================================================================== RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.h,v retrieving revision 1.14 diff -u -p -u -p -r1.14 syslogd.h --- usr.sbin/syslogd/syslogd.h 10 Sep 2014 13:16:20 -0000 1.14 +++ usr.sbin/syslogd/syslogd.h 13 Sep 2014 23:47:50 -0000 @@ -42,7 +42,7 @@ int receive_fd(int); #define MAXUNIX 21 extern int nunix; extern char *path_unix[MAXUNIX]; -extern char *ctlsock_path; +extern char *path_ctlsock; #define dprintf(_f...) do { if (Debug) printf(_f); } while (0) extern int Debug;