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;

Reply via email to