From 22a6165d4be4a0a33679b47e3bea72feebda345a Mon Sep 17 00:00:00 2001 From: Roy Marples Date: Sat, 10 Aug 2019 16:53:01 +0100 Subject: [PATCH] control: Add CTL_FREE_LIST to control memory management. While here, don't try to avoid a malloc and copy of the data because the script env now just has one buffer. --- src/control.c | 152 ++++++++++++++++++++++++++++++-------------------- src/control.h | 15 ++++- 2 files changed, 105 insertions(+), 62 deletions(-) diff --git a/src/control.c b/src/control.c index e263bf6a..6060e897 100644 --- a/src/control.c +++ b/src/control.c @@ -52,27 +52,6 @@ (sizeof(*(su)) - sizeof((su)->sun_path) + strlen((su)->sun_path)) #endif -static void -control_queue_purge(struct dhcpcd_ctx *ctx, char *data) -{ - int found; - struct fd_list *fp; - struct fd_data *fpd; - - /* If no other fd queue has the same data, free it */ - found = 0; - TAILQ_FOREACH(fp, &ctx->control_fds, next) { - TAILQ_FOREACH(fpd, &fp->queue, next) { - if (fpd->data == data) { - found = 1; - break; - } - } - } - if (!found) - free(data); -} - static void control_queue_free(struct fd_list *fd) { @@ -80,14 +59,20 @@ control_queue_free(struct fd_list *fd) while ((fdp = TAILQ_FIRST(&fd->queue))) { TAILQ_REMOVE(&fd->queue, fdp, next); - if (fdp->freeit) - control_queue_purge(fd->ctx, fdp->data); + if (fdp->data_size != 0) + free(fdp->data); free(fdp); } + fd->queue_len = 0; + +#ifdef CTL_FREE_LIST while ((fdp = TAILQ_FIRST(&fd->free_queue))) { TAILQ_REMOVE(&fd->free_queue, fdp, next); + if (fdp->data_size != 0) + free(fdp->data); free(fdp); } +#endif } static void @@ -161,29 +146,33 @@ control_handle1(struct dhcpcd_ctx *ctx, int lfd, unsigned int fd_flags) len = sizeof(run); if ((fd = accept(lfd, (struct sockaddr *)&run, &len)) == -1) - return; + goto error; if ((flags = fcntl(fd, F_GETFD, 0)) == -1 || fcntl(fd, F_SETFD, flags | FD_CLOEXEC) == -1) - { - close(fd); - return; - } + goto error; if ((flags = fcntl(fd, F_GETFL, 0)) == -1 || fcntl(fd, F_SETFL, flags | O_NONBLOCK) == -1) - { - close(fd); - return; - } + goto error; + l = malloc(sizeof(*l)); - if (l) { - l->ctx = ctx; - l->fd = fd; - l->flags = fd_flags; - TAILQ_INIT(&l->queue); - TAILQ_INIT(&l->free_queue); - TAILQ_INSERT_TAIL(&ctx->control_fds, l, next); - eloop_event_add(ctx->eloop, l->fd, control_handle_data, l); - } else + if (l == NULL) + goto error; + + l->ctx = ctx; + l->fd = fd; + l->flags = fd_flags; + TAILQ_INIT(&l->queue); + l->queue_len = 0; +#ifdef CTL_FREE_LIST + TAILQ_INIT(&l->free_queue); +#endif + TAILQ_INSERT_TAIL(&ctx->control_fds, l, next); + eloop_event_add(ctx->eloop, l->fd, control_handle_data, l); + return; + +error: + logerr(__func__); + if (fd != -1) close(fd); } @@ -374,40 +363,85 @@ control_writeone(void *arg) } TAILQ_REMOVE(&fd->queue, data, next); - if (data->freeit) - control_queue_purge(fd->ctx, data->data); - data->data = NULL; /* safety */ - data->data_len = 0; + fd->queue_len--; +#ifdef CTL_FREE_LIST TAILQ_INSERT_TAIL(&fd->free_queue, data, next); +#else + if (data->data_size != 0) + free(data->data); + free(data); +#endif if (TAILQ_FIRST(&fd->queue) == NULL) eloop_event_remove_writecb(fd->ctx->eloop, fd->fd); } int -control_queue(struct fd_list *fd, char *data, size_t data_len, uint8_t fit) +control_queue(struct fd_list *fd, void *data, size_t data_len, bool fit) { struct fd_data *d; - size_t n; - d = TAILQ_FIRST(&fd->free_queue); - if (d) { - TAILQ_REMOVE(&fd->free_queue, d, next); - } else { - n = 0; - TAILQ_FOREACH(d, &fd->queue, next) { - if (++n == CONTROL_QUEUE_MAX) { - errno = ENOBUFS; - return -1; +#ifdef CTL_FREE_LIST + struct fd_data *df; + + d = NULL; + TAILQ_FOREACH(df, &fd->free_queue, next) { + if (!fit) { + if (df->data_size == 0) { + d = df; + break; } + continue; } - d = malloc(sizeof(*d)); + if (d == NULL || d->data_size < df->data_size) { + d = df; + if (d->data_size <= data_len) + break; + } + } + if (d != NULL) + TAILQ_REMOVE(&fd->free_queue, d, next); + else +#endif + { + if (fd->queue_len == CONTROL_QUEUE_MAX) { + errno = ENOBUFS; + return -1; + } + fd->queue_len++; + d = calloc(1, sizeof(*d)); if (d == NULL) return -1; } - d->data = data; + + if (!fit) { +#ifdef CTL_FREE_LIST + if (d->data_size != 0) { + free(d->data); + d->data_size = 0; + } +#endif + d->data = data; + d->data_len = data_len; + goto queue; + } + + if (d->data_size == 0) + d->data = NULL; + if (d->data_size < data_len) { + void *nbuf = realloc(d->data, data_len); + if (nbuf == NULL) { + free(d->data); + free(d); + return -1; + } + d->data = nbuf; + d->data_size = data_len; + } + memcpy(d->data, data, data_len); d->data_len = data_len; - d->freeit = fit; + +queue: TAILQ_INSERT_TAIL(&fd->queue, d, next); eloop_event_add_w(fd->ctx->eloop, fd->fd, control_writeone, fd); return 0; diff --git a/src/control.h b/src/control.h index 795ecebb..e52343db 100644 --- a/src/control.h +++ b/src/control.h @@ -31,14 +31,20 @@ #include "dhcpcd.h" +#if !defined(CTL_FREE_LIST) +#define CTL_FREE_LIST 1 +#elif CTL_FREE_LIST == 0 +#undef CTL_FREE_LIST +#endif + /* Limit queue size per fd */ #define CONTROL_QUEUE_MAX 100 struct fd_data { TAILQ_ENTRY(fd_data) next; - char *data; + void *data; + size_t data_size; size_t data_len; - uint8_t freeit; }; TAILQ_HEAD(fd_data_head, fd_data); @@ -48,7 +54,10 @@ struct fd_list { int fd; unsigned int flags; struct fd_data_head queue; + size_t queue_len; +#ifdef CTL_FREE_LIST struct fd_data_head free_queue; +#endif }; TAILQ_HEAD(fd_list_head, fd_list); @@ -59,7 +68,7 @@ int control_start(struct dhcpcd_ctx *, const char *); int control_stop(struct dhcpcd_ctx *); int control_open(const char *); ssize_t control_send(struct dhcpcd_ctx *, int, char * const *); -int control_queue(struct fd_list *fd, char *data, size_t data_len, uint8_t fit); +int control_queue(struct fd_list *, void *, size_t, bool); void control_close(struct dhcpcd_ctx *ctx); #endif -- 2.47.3