Re: syslogd libevent handler
On Wed, Sep 03, 2014 at 04:34:47PM -0700, Doug Hogan wrote: > On Sun, Aug 31, 2014 at 10:46:50PM +0200, Alexander Bluhm wrote: > > Move the handlers for the poll events into separate functions. They > > will become the libevent callbacks later. > ... > > > + udp_read_handler(pfd[PFD_UNIX_0 + i].fd); > ... > > Shouldn't this be a call to unix_read_handler() instead of > udp_read_handler()? Yes, of course. This bug can be seen in the test output: Sep 04 13:18:17 ??? syslogd-regress[21485]: syslogd regress test log message Sep 04 13:21:35 t430s syslogd-regress[23917]: syslogd regress test log message I have added a check to regress. Thanks for finding this, updated diff below. Index: usr.sbin/syslogd/syslogd.c === RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.c,v retrieving revision 1.121 diff -u -p -r1.121 syslogd.c --- usr.sbin/syslogd/syslogd.c 31 Aug 2014 22:11:43 - 1.121 +++ usr.sbin/syslogd/syslogd.c 4 Sep 2014 11:19:05 - @@ -245,6 +245,13 @@ char *reply_text;/* Start of reply tex size_t ctl_reply_size = 0; /* Number of bytes used in reply */ size_t ctl_reply_offset = 0; /* Number of bytes of reply written so far */ +char *linebuf; +int linesize; + +voidklog_read_handler(int); +voidudp_read_handler(int); +voidunix_read_handler(int); + struct pollfd pfd[N_PFD]; volatile sig_atomic_t MarkSet; @@ -282,12 +289,8 @@ void logto_ctlconn(char *); int main(int argc, char *argv[]) { - int ch, i, linesize, fd; - struct sockaddr_un fromunix; - struct sockaddr_storage from; - socklen_t len; - char *p, *line; - char resolve[MAXHOSTNAMELEN]; + int ch, i, fd; + char *p; int lockpipe[2] = { -1, -1}, pair[2], nullfd; struct addrinfo hints, *res, *res0; FILE *fp; @@ -367,7 +370,7 @@ main(int argc, char *argv[]) if (linesize < MAXLINE) linesize = MAXLINE; linesize++; - if ((line = malloc(linesize)) == NULL) { + if ((linebuf = malloc(linesize)) == NULL) { logerror("Couldn't allocate line buffer"); die(0); } @@ -585,41 +588,13 @@ main(int argc, char *argv[]) } if ((pfd[PFD_KLOG].revents & POLLIN) != 0) { - i = read(pfd[PFD_KLOG].fd, line, linesize - 1); - if (i > 0) { - line[i] = '\0'; - printsys(line); - } else if (i < 0 && errno != EINTR) { - logerror("klog"); - pfd[PFD_KLOG].fd = -1; - pfd[PFD_KLOG].events = 0; - } + klog_read_handler(pfd[PFD_KLOG].fd); } if ((pfd[PFD_INET].revents & POLLIN) != 0) { - len = sizeof(from); - i = recvfrom(pfd[PFD_INET].fd, line, MAXLINE, 0, - (struct sockaddr *)&from, &len); - if (i > 0) { - line[i] = '\0'; - cvthname((struct sockaddr *)&from, resolve, - sizeof(resolve)); - dprintf("cvthname res: %s\n", resolve); - printline(resolve, line); - } else if (i < 0 && errno != EINTR) - logerror("recvfrom inet"); + udp_read_handler(pfd[PFD_INET].fd); } if ((pfd[PFD_INET6].revents & POLLIN) != 0) { - len = sizeof(from); - i = recvfrom(pfd[PFD_INET6].fd, line, MAXLINE, 0, - (struct sockaddr *)&from, &len); - if (i > 0) { - line[i] = '\0'; - cvthname((struct sockaddr *)&from, resolve, - sizeof(resolve)); - dprintf("cvthname res: %s\n", resolve); - printline(resolve, line); - } else if (i < 0 && errno != EINTR) - logerror("recvfrom inet6"); + udp_read_handler(pfd[PFD_INET6].fd); } if ((pfd[PFD_CTLSOCK].revents & POLLIN) != 0) ctlsock_accept_handler(); @@ -630,22 +605,64 @@ main(int argc, char *argv[]) for (i = 0; i < nfunix; i++) { if ((pfd[PFD_UNIX_0 + i].revents & POLLIN) != 0) { - ssize_t rlen; - - len = sizeof(fromunix); - rlen = recvfrom(pfd[PFD_UNIX_0 + i].fd, line, -
Re: syslogd libevent handler
On Sun, Aug 31, 2014 at 10:46:50PM +0200, Alexander Bluhm wrote: > Move the handlers for the poll events into separate functions. They > will become the libevent callbacks later. ... > @@ -631,23 +606,65 @@ main(int argc, char *argv[]) > > for (i = 0; i < nfunix; i++) { > if ((pfd[PFD_UNIX_0 + i].revents & POLLIN) != 0) { > - ssize_t rlen; > - > - len = sizeof(fromunix); > - rlen = recvfrom(pfd[PFD_UNIX_0 + i].fd, line, > - MAXLINE, 0, (struct sockaddr *)&fromunix, > - &len); > - if (rlen > 0) { > - line[rlen] = '\0'; > - printline(LocalHostName, line); > - } else if (rlen == -1 && errno != EINTR) > - logerror("recvfrom unix"); > + udp_read_handler(pfd[PFD_UNIX_0 + i].fd); ... Shouldn't this be a call to unix_read_handler() instead of udp_read_handler()?
Re: syslogd libevent handler
On Sun, Aug 31, 2014 at 10:46:50PM +0200, Alexander Bluhm wrote: > On Fri, Aug 29, 2014 at 11:25:52PM +0200, Alexander Bluhm wrote: > > I will try to pull parts of the diff into separate changes to > > make review easier. > > Move the handlers for the poll events into separate functions. They > will become the libevent callbacks later. > > ok? anyone? > bluhm > > Index: usr.sbin/syslogd/syslogd.c > === > RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.c,v > retrieving revision 1.119 > diff -u -p -r1.119 syslogd.c > --- usr.sbin/syslogd/syslogd.c25 Aug 2014 18:19:18 - 1.119 > +++ usr.sbin/syslogd/syslogd.c31 Aug 2014 20:34:01 - > @@ -245,6 +245,13 @@ char *reply_text;/* Start of reply tex > size_t ctl_reply_size = 0; /* Number of bytes used in reply */ > size_t ctl_reply_offset = 0; /* Number of bytes of reply written so > far */ > > +char *linebuf; > +int linesize; > + > +void klog_read_handler(int); > +void udp_read_handler(int); > +void unix_read_handler(int); > + > struct pollfd pfd[N_PFD]; > > volatile sig_atomic_t MarkSet; > @@ -283,12 +290,8 @@ void logto_ctlconn(char *); > int > main(int argc, char *argv[]) > { > - int ch, i, linesize, fd; > - struct sockaddr_un fromunix; > - struct sockaddr_storage from; > - socklen_t len; > - char *p, *line; > - char resolve[MAXHOSTNAMELEN]; > + int ch, i, fd; > + char *p; > int lockpipe[2] = { -1, -1}, pair[2], nullfd; > struct addrinfo hints, *res, *res0; > FILE *fp; > @@ -368,7 +371,7 @@ main(int argc, char *argv[]) > if (linesize < MAXLINE) > linesize = MAXLINE; > linesize++; > - if ((line = malloc(linesize)) == NULL) { > + if ((linebuf = malloc(linesize)) == NULL) { > logerror("Couldn't allocate line buffer"); > die(0); > } > @@ -586,41 +589,13 @@ main(int argc, char *argv[]) > } > > if ((pfd[PFD_KLOG].revents & POLLIN) != 0) { > - i = read(pfd[PFD_KLOG].fd, line, linesize - 1); > - if (i > 0) { > - line[i] = '\0'; > - printsys(line); > - } else if (i < 0 && errno != EINTR) { > - logerror("klog"); > - pfd[PFD_KLOG].fd = -1; > - pfd[PFD_KLOG].events = 0; > - } > + klog_read_handler(pfd[PFD_KLOG].fd); > } > if ((pfd[PFD_INET].revents & POLLIN) != 0) { > - len = sizeof(from); > - i = recvfrom(pfd[PFD_INET].fd, line, MAXLINE, 0, > - (struct sockaddr *)&from, &len); > - if (i > 0) { > - line[i] = '\0'; > - cvthname((struct sockaddr *)&from, resolve, > - sizeof(resolve)); > - dprintf("cvthname res: %s\n", resolve); > - printline(resolve, line); > - } else if (i < 0 && errno != EINTR) > - logerror("recvfrom inet"); > + udp_read_handler(pfd[PFD_INET].fd); > } > if ((pfd[PFD_INET6].revents & POLLIN) != 0) { > - len = sizeof(from); > - i = recvfrom(pfd[PFD_INET6].fd, line, MAXLINE, 0, > - (struct sockaddr *)&from, &len); > - if (i > 0) { > - line[i] = '\0'; > - cvthname((struct sockaddr *)&from, resolve, > - sizeof(resolve)); > - dprintf("cvthname res: %s\n", resolve); > - printline(resolve, line); > - } else if (i < 0 && errno != EINTR) > - logerror("recvfrom inet6"); > + udp_read_handler(pfd[PFD_INET6].fd); > } > if ((pfd[PFD_CTLSOCK].revents & POLLIN) != 0) > ctlsock_accept_handler(); > @@ -631,23 +606,65 @@ main(int argc, char *argv[]) > > for (i = 0; i < nfunix; i++) { > if ((pfd[PFD_UNIX_0 + i].revents & POLLIN) != 0) { > - ssize_t rlen; > - > - len = sizeof(fromunix); > - rlen = recvfrom(pfd[PFD_UNIX_0 + i].fd, line, > - MAXLINE, 0, (struct sockaddr *)&fromunix, > - &len); > - if (rlen > 0) { > - line[rlen] = '\0'; > - printline(LocalHostName, line);
Re: syslogd libevent handler
On Fri, Aug 29, 2014 at 11:25:52PM +0200, Alexander Bluhm wrote: > I will try to pull parts of the diff into separate changes to > make review easier. Move the handlers for the poll events into separate functions. They will become the libevent callbacks later. ok? bluhm Index: usr.sbin/syslogd/syslogd.c === RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.c,v retrieving revision 1.119 diff -u -p -r1.119 syslogd.c --- usr.sbin/syslogd/syslogd.c 25 Aug 2014 18:19:18 - 1.119 +++ usr.sbin/syslogd/syslogd.c 31 Aug 2014 20:34:01 - @@ -245,6 +245,13 @@ char *reply_text;/* Start of reply tex size_t ctl_reply_size = 0; /* Number of bytes used in reply */ size_t ctl_reply_offset = 0; /* Number of bytes of reply written so far */ +char *linebuf; +int linesize; + +voidklog_read_handler(int); +voidudp_read_handler(int); +voidunix_read_handler(int); + struct pollfd pfd[N_PFD]; volatile sig_atomic_t MarkSet; @@ -283,12 +290,8 @@ void logto_ctlconn(char *); int main(int argc, char *argv[]) { - int ch, i, linesize, fd; - struct sockaddr_un fromunix; - struct sockaddr_storage from; - socklen_t len; - char *p, *line; - char resolve[MAXHOSTNAMELEN]; + int ch, i, fd; + char *p; int lockpipe[2] = { -1, -1}, pair[2], nullfd; struct addrinfo hints, *res, *res0; FILE *fp; @@ -368,7 +371,7 @@ main(int argc, char *argv[]) if (linesize < MAXLINE) linesize = MAXLINE; linesize++; - if ((line = malloc(linesize)) == NULL) { + if ((linebuf = malloc(linesize)) == NULL) { logerror("Couldn't allocate line buffer"); die(0); } @@ -586,41 +589,13 @@ main(int argc, char *argv[]) } if ((pfd[PFD_KLOG].revents & POLLIN) != 0) { - i = read(pfd[PFD_KLOG].fd, line, linesize - 1); - if (i > 0) { - line[i] = '\0'; - printsys(line); - } else if (i < 0 && errno != EINTR) { - logerror("klog"); - pfd[PFD_KLOG].fd = -1; - pfd[PFD_KLOG].events = 0; - } + klog_read_handler(pfd[PFD_KLOG].fd); } if ((pfd[PFD_INET].revents & POLLIN) != 0) { - len = sizeof(from); - i = recvfrom(pfd[PFD_INET].fd, line, MAXLINE, 0, - (struct sockaddr *)&from, &len); - if (i > 0) { - line[i] = '\0'; - cvthname((struct sockaddr *)&from, resolve, - sizeof(resolve)); - dprintf("cvthname res: %s\n", resolve); - printline(resolve, line); - } else if (i < 0 && errno != EINTR) - logerror("recvfrom inet"); + udp_read_handler(pfd[PFD_INET].fd); } if ((pfd[PFD_INET6].revents & POLLIN) != 0) { - len = sizeof(from); - i = recvfrom(pfd[PFD_INET6].fd, line, MAXLINE, 0, - (struct sockaddr *)&from, &len); - if (i > 0) { - line[i] = '\0'; - cvthname((struct sockaddr *)&from, resolve, - sizeof(resolve)); - dprintf("cvthname res: %s\n", resolve); - printline(resolve, line); - } else if (i < 0 && errno != EINTR) - logerror("recvfrom inet6"); + udp_read_handler(pfd[PFD_INET6].fd); } if ((pfd[PFD_CTLSOCK].revents & POLLIN) != 0) ctlsock_accept_handler(); @@ -631,23 +606,65 @@ main(int argc, char *argv[]) for (i = 0; i < nfunix; i++) { if ((pfd[PFD_UNIX_0 + i].revents & POLLIN) != 0) { - ssize_t rlen; - - len = sizeof(fromunix); - rlen = recvfrom(pfd[PFD_UNIX_0 + i].fd, line, - MAXLINE, 0, (struct sockaddr *)&fromunix, - &len); - if (rlen > 0) { - line[rlen] = '\0'; - printline(LocalHostName, line); - } else if (rlen == -1 && errno != EINTR) - logerror("recvfrom unix"); +