commit 56486cec409e44d26b74a7f6716196a717abd189
parent 5ddc5156392cf6d98b654ec66b27f6912524030b
Author: Jacob R. Edwards <n/a>
Date: Sat, 8 Oct 2022 15:02:33 -0700
Use a common backend for client and server writing
Both the server and client now use the new bufword and bufline
functions as a backend for building a buffer to write to each other.
Buffers have had other changes and additions too:
- size_t is used instead of unsigned int
- Add bufenlarge function to ensure the buffer can at least fit N
more bytes
The client was modified to support multi-word responses aswell,
albeit it's way of doing so isn't ideal (there isn't any way to
tell multi-word lines apart from lines with words containing quoted
words). Additionally errors from printing responses are now propagated.
There were some bugs which were fixed:
- Fix player listing in player command incrementing the player's
argument pointer
- Fix apunquote NULL dereference when given a NULL string (say,
when exhaustively unquoting a string)
And finally, the terrible server response structure and functions
were removed (replaced, as alluded to above, with a simple buffer
and the new bufword and bufline functions). (Made easier as one of
the reasons the response struct was created was made moot by the
polling fix--to poll read or write as needed--was introduced.)
Diffstat:
15 files changed, 266 insertions(+), 229 deletions(-)
diff --git a/apc/apc.c b/apc/apc.c
@@ -1,5 +1,5 @@
/*
- * Copyright 2021 Jacob R. Edwards
+ * Copyright 2021, 2022 Jacob R. Edwards
*
* ap -- audio player
*
@@ -41,12 +41,29 @@ die(char *s)
}
int
+printline(char *buf)
+{
+ char *p;
+
+ p = apunquote(&buf, NULL);
+ if (p && !buf)
+ return puts(p) < 0;
+
+ if (!(p = apquote(p, NULL)))
+ return 1;
+ return printf("%s %s\n", p, buf) < 0;
+}
+
+int
printresponse(struct apc *apc)
{
char *buf;
while ((buf = apc_read(apc))) {
- puts(buf);
+ if (printline(buf)) {
+ free(buf);
+ return 1;
+ }
free(buf);
}
if (apc->status == NULL)
@@ -92,8 +109,8 @@ xargs(struct apc *apc, FILE *fp)
while ((len = getline(&buf, &size, stdin)) >= 0) {
if (len && buf[len - 1] == '\n')
buf[--len] = 0;
- if (apc_writeword(apc, buf))
- return "unable to write to server";
+ if (apc_bufword(apc, buf))
+ return "unable to add word";
}
free(buf);
if (ferror(stdin))
@@ -135,11 +152,11 @@ main(int argc, char *argv[])
err = NULL;
for (i = 0; !err && i < argc; ++i)
- if (apc_writeword(apc, argv[i]))
- err = "unable to write argument";
+ if (apc_bufword(apc, argv[i]))
+ err = "unable to write word";
if (!err && (!insert || !(err = xargs(apc, stdin)))) {
- if (apc_write(apc, "\n", 1))
- err = "unable to terminate command";
+ if (apc_write(apc))
+ err = "unable to write command";
else if (printresponse(apc))
err = "unable print response";
}
diff --git a/aps/Makefile b/aps/Makefile
@@ -1,6 +1,6 @@
name = aps
-src = aps.c arg.c bug.c command.c find.c log.c main.c player.c \
- queue.c response.c split.c
+src = aps.c arg.c buf.c bug.c command.c find.c log.c main.c player.c \
+ queue.c split.c
inc = ${src:.c=.h} util.h
ldlibs = ../lib/ap/libap.a
cflags = -I../lib
diff --git a/aps/aps.c b/aps/aps.c
@@ -31,6 +31,7 @@
#include "aps.h"
#include "arg.h"
+#include "buf.h"
#include "bug.h"
#include "command.h"
#include "find.h"
@@ -95,7 +96,7 @@ aps_drop(struct aps *aps, int fd)
aps_log(aps, INFO, fd, "dropping client");
--aps->nfds;
aps->pfds[fd].fd = -1;
- respfree(aps, fd);
+ buffree(aps->clients[fd].response);
buffree(aps->clients[fd].request);
return sclose(fd);
}
@@ -103,12 +104,10 @@ aps_drop(struct aps *aps, int fd)
int
aps_addfd(struct aps *aps, int fd)
{
- if ((aps->clients[fd].request = bufnew()) == NULL) {
- buffree(aps->clients[fd].request);
+ if ((aps->clients[fd].request = bufnew()) == NULL)
return 1;
- }
- if (respnew(aps, fd)) {
- respfree(aps, fd);
+ if ((aps->clients[fd].response = bufnew()) == NULL) {
+ buffree(aps->clients[fd].request);
return 1;
}
@@ -143,6 +142,32 @@ aps_accept(struct aps *aps)
}
int
+aps_respond(struct aps *aps, int fd)
+{
+ ssize_t n;
+ struct buf *buf;
+
+ buf = aps->clients[fd].response;
+ n = send(fd, buf->data, buf->len, MSG_NOSIGNAL);
+ switch (n) {
+ case 0:
+ errno = ECONNRESET;
+ /* no break */
+ case -1:
+ return 1;
+ default:
+ if ((buf->len - n) == 0) {
+ buf->len = 0;
+ aps->pfds[fd].events = POLLIN;
+ } else {
+ memmove(buf->data, buf->data + n, buf->len - n);
+ buf->len -= n;
+ }
+ return 0;
+ }
+}
+
+int
aps_handle(struct aps *aps, int fd)
{
if (aps->pfds[fd].revents & (POLLERR | POLLHUP | POLLNVAL)) {
@@ -151,7 +176,7 @@ aps_handle(struct aps *aps, int fd)
if (aps_command(aps, fd))
return 1;
} else if (aps->pfds[fd].revents & POLLOUT) {
- if (respond(aps, fd))
+ if (aps_respond(aps, fd))
return 1;
} else {
bug("impossible poll event");
@@ -236,7 +261,7 @@ aps_command(struct aps *aps, int s)
buf = aps->clients[s].request;
aps_logfmt(aps, COMMAND, s, "input '%s'", buf->data);
- argv = asplit(buf->data, isspace);
+ argv = asplit(buf->data, NULL);
if (argv == NULL)
return 1;
if (*argv == NULL) {
@@ -259,7 +284,7 @@ exit:
buf->len -= len;
free(argv);
- return respfinish(aps, s, status);
+ return aps_bufstatus(aps, s, status);
}
void
diff --git a/aps/aps.h b/aps/aps.h
@@ -3,11 +3,9 @@
struct aps;
#include "player.h"
-#include "response.h"
struct client {
- struct buf *request;
- struct response *resp;
+ struct buf *request, *response;
};
struct aps {
@@ -28,6 +26,7 @@ struct aps *aps_open(char *, char **);
void aps_close(struct aps *);
int aps_drop(struct aps *, int);
int aps_accept(struct aps *);
+int aps_respond(struct aps *, int);
int aps_handle(struct aps *, int);
int aps_named(struct aps *);
int aps_update(struct aps *);
diff --git a/aps/buf.c b/aps/buf.c
@@ -0,0 +1,52 @@
+#include <sys/socket.h>
+
+#include <ctype.h>
+#include <limits.h>
+#include <poll.h>
+#include <stddef.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include "aps.h"
+#include "log.h"
+
+int
+aps_bufword(struct aps *aps, int fd, char *word)
+{
+ return bufword(aps->clients[fd].response, word);
+}
+
+int
+aps_bufline(struct aps *aps, int fd, char *line)
+{
+ return bufline(aps->clients[fd].response, line);
+}
+
+int
+aps_bufstatus(struct aps *aps, int fd, char *error)
+{
+ size_t errlen;
+ struct buf *buf;
+
+ buf = aps->clients[fd].response;
+ if (bufline(buf, NULL))
+ return 1;
+
+ if (!error) {
+ if (bufappend(buf, "ok\n", 3))
+ return 1;
+ aps->pfds[fd].events = POLLOUT;
+ return 0;
+ }
+
+ errlen = strlen(error);
+ if (bufenlarge(buf, 7 + errlen + 1))
+ return 1;
+ bufappend(buf, "error: ", 7);
+ bufappend(buf, error, errlen);
+ bufappend(buf, "\n", 1);
+ aps->pfds[fd].events = POLLOUT;
+
+ aps_logfmt(aps, COMMAND, fd, "error: %s", error);
+ return 0;
+}
diff --git a/aps/buf.h b/aps/buf.h
@@ -0,0 +1,3 @@
+int aps_bufword(struct aps *, int, char *);
+int aps_bufline(struct aps *, int, char *);
+int aps_bufstatus(struct aps *, int, char *);
diff --git a/aps/command.c b/aps/command.c
@@ -28,6 +28,7 @@
#include "aps.h"
#include "arg.h"
+#include "buf.h"
#include "bug.h"
#include "command.h"
#include "find.h"
@@ -93,7 +94,7 @@ com_list(struct aps *aps, int fd, int argc, char **argv)
return NULL;
while ((item = findnext(&search))) {
- if (respadd(aps, fd, item->path))
+ if (aps_bufline(aps, fd, item->path))
return errstr;
}
stopsearch(&search);
@@ -154,7 +155,7 @@ com_name(struct aps *aps, int fd, int argc, char **argv)
{
if (!aps->queue)
return "Empty queue";
- if (respadd(aps, fd, aps->queue->path))
+ if (aps_bufline(aps, fd, aps->queue->path))
return errstr;
return NULL;
}
@@ -207,7 +208,7 @@ com_status(struct aps *aps, int fd, int argc, char **argv)
bug("impossible player state");
}
- if (respadd(aps, fd, status))
+ if (aps_bufstatus(aps, fd, status))
return errstr;
return NULL;
}
@@ -232,7 +233,7 @@ com_log(struct aps *aps, int fd, int argc, char **argv)
char *
com_player(struct aps *aps, int fd, int argc, char **argv)
{
- char **ap;
+ int i;
struct player *new;
if (argc) {
@@ -247,9 +248,8 @@ com_player(struct aps *aps, int fd, int argc, char **argv)
return NULL;
}
- /* TODO: Send on a single line, quoted if required */
- for (ap = aps->player->argv; *ap; ++ap)
- if (respadd(aps, fd, *ap))
+ for (i = 0; aps->player->argv[i]; ++i)
+ if (aps_bufword(aps, fd, aps->player->argv[i]))
return errstr;
return NULL;
}
@@ -279,7 +279,7 @@ com_commands(struct aps *aps, int fd, int argc, char **argv)
int i;
for (i = 0; i < aps->ncoms; ++i)
- if (respadd(aps, fd, aps->coms[i].name))
+ if (aps_bufline(aps, fd, aps->coms[i].name))
return errstr;
return NULL;
}
diff --git a/aps/response.c b/aps/response.c
@@ -1,125 +0,0 @@
-/*
- * Copyright 2021 Jacob R. Edwards
- *
- * ap -- audio player
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program. If not, see <https://www.gnu.org/licenses/>.
- */
-
-#include <sys/socket.h>
-
-#include <errno.h>
-#include <limits.h>
-#include <poll.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-
-#include "aps.h"
-#include "log.h"
-#include "util.h"
-
-#define RESP (aps->clients[fd].resp)
-
-int
-respnew(struct aps *aps, int fd)
-{
- if ((RESP = calloc(1, sizeof(struct response))) == NULL)
- return 1;
- if ((RESP->buf = bufnew()) == NULL) {
- free(RESP);
- return 1;
- }
- return 0;
-}
-
-void
-respfree(struct aps *aps, int fd)
-{
- if (RESP == NULL)
- return;
- buffree(RESP->buf);
- free(RESP);
- RESP = NULL;
-}
-
-int
-allquote(int c)
-{
- return 1;
-}
-
-int
-respadd(struct aps *aps, int fd, char *item)
-{
- int status;
-
- item = apquote(item, (strcmp(item, "ok") ? NULL : allquote));
- if (item == NULL)
- return 1;
-
- status = (bufappend(RESP->buf, item, strlen(item)) ||
- bufappend(RESP->buf, "\n", 1));
- free(item);
-
- return status;
-}
-
-int
-respfinish(struct aps *aps, int fd, char *error)
-{
- if (error == NULL) {
- /* NOTE: Perhaps it should return an error? */
- if (bufappend(RESP->buf, "ok\n", 3))
- respfinish(aps, fd, errstr);
- } else {
- aps_logfmt(aps, COMMAND, fd, "error '%s'", error);
- aps->clients[fd].resp->buf->len = 0;
- if (bufappend(RESP->buf, "error: ", 7) ||
- bufappend(RESP->buf, error, strlen(error)) ||
- bufappend(RESP->buf, "\n", 1))
- return 1;
- }
-
- aps->clients[fd].resp->lock = 1;
- aps->pfds[fd].events = POLLOUT;
- return 0;
-}
-
-int
-respond(struct aps *aps, int fd)
-{
- ssize_t n;
- struct response *resp;
-
- resp = RESP;
- if (!resp->lock || resp->buf->len - resp->sent == 0)
- return 1;
- n = send(fd, resp->buf->data + resp->sent, resp->buf->len - resp->sent,
- MSG_NOSIGNAL);
-
- if (n < 0)
- return 1;
- if (n == 0) {
- errno = ECONNRESET;
- return 1;
- }
- if (resp->sent += n == resp->buf->len) {
- resp->buf->len = 0;
- resp->sent = 0;
- resp->lock = 0;
- aps->pfds[fd].events = POLLIN;
- }
- return 0;
-}
diff --git a/aps/response.h b/aps/response.h
@@ -1,11 +0,0 @@
-struct response {
- int lock;
- struct buf *buf;
- unsigned int sent;
-};
-
-int respnew(struct aps *, int);
-void respfree(struct aps *, int);
-int respadd(struct aps *, int, char *);
-int respfinish(struct aps *, int, char *);
-int respond(struct aps *, int);
diff --git a/aps/split.c b/aps/split.c
@@ -30,11 +30,11 @@ split(char **ap, unsigned int len, char *s, int (*sep)(int))
char *p;
unsigned int i;
- for (i = 0; (p = apunquote(&s, sep)) && p[0]; ++i) {
+ for (i = 0; (p = apunquote(&s, sep)); ++i) {
if (i < len)
ap[i] = p;
}
- if (!p)
+ if (s)
return -1;
if (len)
diff --git a/lib/ap/buf.c b/lib/ap/buf.c
@@ -1,5 +1,5 @@
/*
- * Copyright 2021 Jacob R. Edwards
+ * Copyright 2021, 2022 Jacob R. Edwards
*
* ap -- audio player
*
@@ -20,8 +20,11 @@
#include <errno.h>
#include <stdlib.h>
#include <string.h>
+#include <stdint.h>
+#include <ctype.h>
#include <ap/buf.h>
+#include <ap/quote.h>
struct buf *
bufnew(void)
@@ -39,7 +42,7 @@ buffree(struct buf *buf)
}
int
-bufresize(struct buf *buf, unsigned int newsize)
+bufresize(struct buf *buf, size_t newsize)
{
char *tmp;
@@ -58,12 +61,71 @@ bufresize(struct buf *buf, unsigned int newsize)
}
int
-bufappend(struct buf *buf, void *data, unsigned int len)
+bufenlarge(struct buf *buf, size_t needed)
{
- if ((buf == NULL || buf->size - buf->len < len) &&
- bufresize(buf, (buf->size * 2) + len) != 0)
+ size_t newsize;
+
+ for (newsize = buf->size ? buf->size : (1024 * 8);
+ newsize != SIZE_MAX && needed > (newsize - buf->len);) {
+ if (newsize > SIZE_MAX / 4)
+ newsize = SIZE_MAX;
+ else
+ newsize *= 4;
+ }
+
+ if (needed > (newsize - buf->len)) {
+ errno = EOVERFLOW;
+ return 1;
+ }
+ if (newsize == buf->size)
+ return 0;
+ return bufresize(buf, newsize);
+}
+
+int
+bufappend(struct buf *buf, void *data, size_t len)
+{
+ if (buf == NULL || bufenlarge(buf, len))
return 1;
memcpy(buf->data + buf->len, data, len);
buf->len += len;
return 0;
}
+
+int
+bufword(struct buf *buf, char *word)
+{
+ size_t len;
+
+ word = apquote(word, NULL);
+ if (word == NULL)
+ return 1;
+ len = strlen(word);
+
+ if (bufenlarge(buf, len + 1)) {
+ free(word);
+ return 1;
+ }
+
+ if (buf->len && !isspace(buf->data[buf->len - 1]))
+ bufappend(buf, " ", 1);
+ bufappend(buf, word, len);
+
+ free(word);
+ return 0;
+}
+
+int
+bufline(struct buf *buf, char *line)
+{
+ if (buf->len && buf->data[buf->len - 1] != '\n' &&
+ bufappend(buf, "\n", 1))
+ return 1;
+
+ if (!line)
+ return 0;
+
+ if (bufword(buf, line))
+ return 1;
+ return bufappend(buf, "\n", 1);
+}
diff --git a/lib/ap/buf.h b/lib/ap/buf.h
@@ -1,10 +1,13 @@
struct buf {
- unsigned int len;
- unsigned int size;
char *data;
+ size_t len;
+ size_t size;
};
struct buf *bufnew(void);
void buffree(struct buf *);
-int bufresize(struct buf *, unsigned int);
-int bufappend(struct buf *, void *, unsigned int);
+int bufresize(struct buf *, size_t);
+int bufenlarge(struct buf *, size_t);
+int bufappend(struct buf *, void *, size_t);
+int bufword(struct buf *, char *);
+int bufline(struct buf *, char *);
diff --git a/lib/ap/client.c b/lib/ap/client.c
@@ -20,6 +20,7 @@
#include <sys/socket.h>
#include <ctype.h>
+#include <errno.h>
#include <stdlib.h>
#include <string.h>
@@ -33,7 +34,8 @@ apc_free(struct apc *apc)
free(apc->status);
apclose(apc->con);
- buffree(apc->buf);
+ buffree(apc->request);
+ buffree(apc->response);
free(apc);
}
@@ -42,7 +44,9 @@ apc_new(char *path)
{
struct apc *apc;
- if ((apc = calloc(1, sizeof(*apc))) == NULL || (apc->buf = bufnew()) == NULL ||
+ if ((apc = calloc(1, sizeof(*apc))) == NULL ||
+ (apc->request = bufnew()) == NULL ||
+ (apc->response = bufnew()) == NULL ||
(apc->con = apopen(path, connect, 0)) == NULL) {
apc_free(apc);
return NULL;
@@ -51,26 +55,27 @@ apc_new(char *path)
}
int
-apc_write(struct apc *apc, char *buf, size_t len)
+apc_bufword(struct apc *apc, char *word)
{
- if (send(apc->con->sock, buf, len, 0) != len)
- return 1;
- apc->lastbyte = buf[len - 1];
- return 0;
+ return bufword(apc->request, word);
}
int
-apc_writeword(struct apc *apc, char *word)
+apc_write(struct apc *apc)
{
- int status;
+ ssize_t n;
+ size_t offset;
+ struct buf *buf;
- if (!isspace(apc->lastbyte) && apc_write(apc, " ", 1))
- return 1;
- if (!(word = apquote(word, NULL)))
+ buf = apc->request;
+ if (bufline(buf, NULL))
return 1;
- status = apc_write(apc, word, strlen(word));
- free(word);
- return status;
+
+ offset = 0;
+ while (apc->request->len - offset &&
+ (n = send(apc->con->sock, buf->data + offset, buf->len - offset, 0)) > 0)
+ offset += n;
+ return n <= 0;
}
int
@@ -82,30 +87,31 @@ apc_recv(struct apc *apc)
len = recv(apc->con->sock, buf, sizeof(buf), 0);
if (len <= 0)
return 1;
- return bufappend(apc->buf, buf, len);
+ return bufappend(apc->response, buf, len);
}
char *
apc_read(struct apc *apc)
{
- unsigned int len;
+ size_t offset, len;
char *buf, *end, *tmp;
- while ((end = memchr(apc->buf->data, '\n', apc->buf->len)) == NULL &&
- apc_recv(apc) == 0)
- ;
- if (end == NULL)
- return NULL;
+ for (offset = 0; (end = memchr(apc->response->data + offset, '\n',
+ apc->response->len - offset)) == NULL;) {
+ offset += apc->response->len;
+ if (apc_recv(apc))
+ return NULL;
+ }
- len = end - apc->buf->data + 1;
- buf = malloc(len);
+ len = end - apc->response->data;
+ buf = malloc(len + 1);
if (buf == NULL)
return NULL;
- memcpy(buf, apc->buf->data, len);
- buf[len - 1] = 0;
+ memcpy(buf, apc->response->data, len);
+ buf[len] = 0;
- memmove(apc->buf->data, apc->buf->data + len, apc->buf->len - len);
- apc->buf->len -= len;
+ memmove(apc->response->data, end + 1,
+ apc->response->len -= len + 1);
if (strcmp(buf, "ok") == 0 || strncmp(buf, "error: ", 7) == 0) {
if (*buf == 'e')
@@ -116,10 +122,5 @@ apc_read(struct apc *apc)
return NULL;
}
- if (*buf == '\'') {
- tmp = buf;
- tmp = apunquote(&tmp, NULL);
- strcpy(buf, tmp);
- }
return buf;
}
diff --git a/lib/ap/client.h b/lib/ap/client.h
@@ -1,13 +1,12 @@
struct apc {
char *status;
struct apcon *con;
- struct buf *buf;
- char lastbyte;
+ struct buf *request, *response;
};
void apc_free(struct apc *);
struct apc *apc_new(char *);
-int apc_write(struct apc *, char *, size_t);
-int apc_writeword(struct apc *, char *);
+int apc_bufword(struct apc *, char *);
+int apc_write(struct apc *);
int apc_recv(struct apc *);
char *apc_read(struct apc *);
diff --git a/lib/ap/quote.c b/lib/ap/quote.c
@@ -23,13 +23,10 @@
#include <string.h>
int
-apneedquote(char *s, int (*needquote)(int))
+apneedquote(int c, int (*needquote)(int))
{
- if (*s == 0)
- return 1;
- while (!iscntrl(*s) && !isspace(*s) && *s != '\'' && (!needquote || !needquote(*s)))
- ++s;
- return *s;
+ return c == '\0' || iscntrl(c) || isspace(c) || c == '\'' ||
+ (needquote && needquote(c));
}
char *
@@ -38,8 +35,12 @@ apquote(char *s, int (*needquote)(int))
char *buf, *p;
size_t i;
- if (!apneedquote(s, needquote))
- return strdup(s);
+ if (s[0]) {
+ for (i = 0; !apneedquote(s[i], needquote); ++i)
+ ;
+ if (!s[i])
+ return strdup(s);
+ }
buf = malloc(strlen(s) * 2 + 3);
if (buf == NULL)
@@ -58,6 +59,12 @@ apquote(char *s, int (*needquote)(int))
return buf;
}
+int
+apissep(int c, int (*issep)(int))
+{
+ return c != '\'' && apneedquote(c, issep);
+}
+
char *
apunquote(char **sp, int (*issep)(int))
{
@@ -65,17 +72,17 @@ apunquote(char **sp, int (*issep)(int))
int quoted;
size_t len;
- if (!issep)
- issep = isspace;
+ if (!*sp)
+ return NULL;
s = *sp;
- while (s[0] && issep(s[0]))
+ while (s[0] && apissep(s[0], issep))
++s;
p = s;
len = strlen(p);
quoted = 0;
- while (s[0] && (quoted || !issep(s[0]))) {
+ while (s[0] && (quoted || !apissep(s[0], issep))) {
if (s[0] == '\'') {
if (!quoted)
quoted = 1;
@@ -93,7 +100,12 @@ apunquote(char **sp, int (*issep)(int))
return NULL;
}
- s[0] = '\0';
- *sp = s + 1;
- return p;
+ if (s[0]) {
+ s[0] = '\0';
+ *sp = s + 1;
+ return p;
+ }
+
+ *sp = NULL;
+ return p[0] ? p : NULL;
}
