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");
+
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 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 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,
-
