commit c76476dd595e09f6dc307ad3a45e4ef834c9b754
parent 0292e15089d39b9afc7f81a99b51de44c5c34314
Author: Jacob R. Edwards <jacobouno@protonmail.com>
Date: Thu, 15 Jul 2021 18:39:26 -0700
Improve memory management
- Fully initialize aps struct in aps_open
- Implement argument array helper functions
- Dynamically allocate next member of aps struct
- Dynamically allocate player struct
It's easier to manage in a way since the struct is either NULL
or fully initialized.
- Allow NULL pointer in apclose
- Free aps queue in aps_close
Diffstat:
12 files changed, 160 insertions(+), 68 deletions(-)
diff --git a/aps/aps.c b/aps/aps.c
@@ -22,14 +22,17 @@
#include <limits.h>
#include <poll.h>
#include <stdlib.h>
+#include <string.h>
#include "aps.h"
+#include "arg.h"
#include "command.h"
#include "config.h"
+#include "queue.h"
#include "util.h"
struct aps *
-aps_open(char *path)
+aps_open(char *path, char **player, char **next)
{
struct aps *aps;
int i;
@@ -38,17 +41,28 @@ aps_open(char *path)
if (aps == NULL)
return NULL;
- aps->con = apopen(path, bind, SOCK_NONBLOCK);
- if (aps->con == NULL || listen(aps->con->sock, 10)) {
- free(aps);
- return NULL;
- }
-
for (i = 0; i < LEN(aps->pfds); ++i) {
aps->pfds[i].fd = -1;
aps->pfds[i].events = POLLIN | POLLOUT;
}
+ aps->player = pnew(player[0], player + 1);
+ if (aps->player == NULL) {
+ aps_close(aps);
+ return NULL;
+ }
+
+ if (next && argsub(&aps->next, next, arglen(next)) == NULL) {
+ aps_close(aps);
+ return NULL;
+ }
+
+ aps->con = apopen(path, bind, SOCK_NONBLOCK);
+ if (aps->con == NULL || listen(aps->con->sock, 10)) {
+ aps_close(aps);
+ return NULL;
+ }
+
return aps;
}
@@ -57,14 +71,21 @@ aps_close(struct aps *aps)
{
int i;
- for (i = 0; i < LEN(aps->next); ++i)
- free(aps->next[i]);
+ if (aps == NULL)
+ return;
+
for (i = 0; i < LEN(aps->pfds); ++i)
if (aps->pfds[i].fd != -1)
aps_drop(aps, i);
+
apclose(aps->con);
- pstop(&aps->player);
- free(aps->player.path);
+ pfree(aps->player);
+
+ for (i = 0; i < arglen(aps->next); ++i)
+ free(aps->next[i]);
+ while (aps->queue)
+ queue_remove(aps, aps->queue);
+ free(aps);
}
int
diff --git a/aps/aps.h b/aps/aps.h
@@ -8,8 +8,7 @@ struct aps;
#include "response.h"
struct aps {
- char *next[AP_ARGV_MAX];
- int nextlen;
+ char **next;
int close;
int logmask;
int nfds;
@@ -18,12 +17,12 @@ struct aps {
struct buf *request;
struct response *resp;
} clients[OPEN_MAX];
- struct player player;
+ struct player *player;
struct pollfd pfds[OPEN_MAX];
struct item *queue;
};
-struct aps *aps_open(char *);
+struct aps *aps_open(char *, char **, char **);
void aps_close(struct aps *);
int aps_drop(struct aps *, int);
int aps_accept(struct aps *);
diff --git a/aps/arg.c b/aps/arg.c
@@ -0,0 +1,61 @@
+#include <string.h>
+#include <stdlib.h>
+
+#include "util.h"
+
+unsigned int
+arglen(char **args)
+{
+ unsigned int i;
+
+ for (i = 0; args && args[i] != NULL; ++i)
+ ;
+ return i;
+}
+
+char **
+argdup(char **args, unsigned int len)
+{
+ char **nargs;
+ unsigned int i;
+
+ nargs = calloc(len + 1, sizeof(*args));
+ if (nargs == NULL)
+ return NULL;
+
+ for (i = 0; i < len; ++i) {
+ ASSERT(args[i] != NULL);
+ nargs[i] = strdup(args[i]);
+ if (nargs[i] == NULL)
+ break;
+ }
+
+ if (i == len)
+ return nargs;
+
+ for (i = 0; nargs[i] != NULL; ++i)
+ free(nargs[i]);
+ free(nargs);
+ return NULL;
+}
+
+void
+argfree(char **args)
+{
+ unsigned int len, i;
+
+ len = arglen(args);
+ for (i = 0; i < len; ++i)
+ free(args[i]);
+ free(args);
+}
+
+char **
+argsub(char ***p, char **args, unsigned int len)
+{
+ args = argdup(args, len);
+ if (args == NULL)
+ return NULL;
+ *p = args;
+ return args;
+}
diff --git a/aps/arg.h b/aps/arg.h
@@ -0,0 +1,4 @@
+unsigned int arglen(char **);
+char **argdup(char **, unsigned int);
+void argfree(char **);
+char **argsub(char ***, char **, unsigned int);
diff --git a/aps/command.c b/aps/command.c
@@ -25,6 +25,7 @@
#include <string.h>
#include "aps.h"
+#include "arg.h"
#include "find.h"
#include "log.h"
#include "queue.h"
@@ -113,7 +114,7 @@ aps_play(struct aps *aps, int s, int argc, char **argv)
if (aps->queue == NULL)
return eposition;
- if (pplay(&aps->player, aps->queue->path))
+ if (pplay(aps->player, aps->queue->path))
return strerror(errno);
return NULL;
}
@@ -121,7 +122,7 @@ aps_play(struct aps *aps, int s, int argc, char **argv)
char *
aps_pause(struct aps *aps, int s, int argc, char **argv)
{
- if (psuspend(&aps->player))
+ if (psuspend(aps->player))
return strerror(errno);
return NULL;
}
@@ -129,7 +130,7 @@ aps_pause(struct aps *aps, int s, int argc, char **argv)
char *
aps_stop(struct aps *aps, int s, int argc, char **argv)
{
- if (pstop(&aps->player))
+ if (pstop(aps->player))
return strerror(errno);
return NULL;
}
@@ -137,7 +138,7 @@ aps_stop(struct aps *aps, int s, int argc, char **argv)
char *
aps_toggle(struct aps *aps, int s, int argc, char **argv)
{
- if (ptoggle(&aps->player))
+ if (ptoggle(aps->player))
return strerror(errno);
return NULL;
}
@@ -159,21 +160,12 @@ aps_name(struct aps *aps, int s, int argc, char **argv)
char *
aps_next(struct aps *aps, int s, int argc, char **argv)
{
- int i;
struct item *n;
- if (argc) {
- for (i = 0; i < argc; ++i) {
- free(aps->next[i]);
- aps->next[i] = strdup(argv[i]);
- if (aps->next[i] == NULL)
- return strerror(errno);
- }
- aps->nextlen = i;
- return NULL;
- }
+ if (argc && argsub(&aps->next, argv, argc) == NULL)
+ return strerror(errno);
- n = find(aps->queue, aps->queue, aps->next, aps->nextlen, 0);
+ n = find(aps->queue, aps->queue, aps->next, arglen(aps->next), 0);
if (n == NULL || queue_set(aps, n))
return strerror(errno);
return NULL;
@@ -184,7 +176,7 @@ aps_previous(struct aps *aps, int s, int argc, char **argv)
{
struct item *n;
- n = find(aps->queue, aps->queue, aps->next, aps->nextlen, FIND_REVERSE);
+ n = find(aps->queue, aps->queue, aps->next, arglen(aps->next), FIND_REVERSE);
if (n == NULL || queue_set(aps, n))
return strerror(errno);
return NULL;
@@ -221,7 +213,7 @@ aps_status(struct aps *aps, int s, int argc, char **argv)
{
char *status;
- switch (aps->player.state) {
+ switch (aps->player->state) {
case OFF:
status = "off";
break;
diff --git a/aps/config.h b/aps/config.h
@@ -5,7 +5,7 @@ static char *player[] = {
};
static char *next[] = {
- "1"
+ "1", NULL
};
static int timeout = 250;
diff --git a/aps/main.c b/aps/main.c
@@ -39,8 +39,8 @@ sigchld(int sig)
while ((pid =
waitpid(-1, &status, WCONTINUED | WNOHANG | WUNTRACED)) > 0) {
- if (pid == aps->player.pid)
- pupdate(&aps->player, status);
+ if (pid == aps->player->pid)
+ pupdate(aps->player, status);
}
}
@@ -48,7 +48,7 @@ int
run(struct aps *aps)
{
while (!aps->close) {
- if (!aps->player.state && aps->queue)
+ if (!aps->player->state && aps->queue)
aps_next(aps, -1, 0, NULL);
if ((aps_accept(aps) || aps_update(aps)) && errno != EINTR)
return 1;
@@ -56,33 +56,6 @@ run(struct aps *aps)
return 0;
}
-struct aps *
-setup(char *path)
-{
- struct aps *aps;
-
- aps = aps_open(path);
- if (aps == NULL)
- return NULL;
-
- signal(SIGCHLD, sigchld);
- signal(SIGINT, sigclose);
- signal(SIGTERM, sigclose);
-
- if (aps_next(aps, -1, LEN(next), next)) {
- aps_close(aps);
- return NULL;
- }
-
- aps->player.argv = player + 1;
- aps->player.prog = player[0];
- aps->player.state = STOPPED;
-
- aps->logmask = INFO | WARN | ERROR | FATAL;
-
- return aps;
-}
-
int
main(int argc, char *argv[])
{
@@ -95,12 +68,19 @@ main(int argc, char *argv[])
}
#endif
- aps = setup(NULL);
+ aps = aps_open(NULL, player, next);
if (aps == NULL) {
slog("unable to setup server");
return 1;
}
+ aps->player->state = STOPPED;
+ aps->logmask = INFO | WARN | ERROR | FATAL;
+
+ signal(SIGCHLD, sigchld);
+ signal(SIGINT, sigclose);
+ signal(SIGTERM, sigclose);
+
error = run(aps);
if (error == 0)
error = aps->close;
diff --git a/aps/mkfile b/aps/mkfile
@@ -1,7 +1,7 @@
# Copyright 2021 Jacob R. Edwards
name = aps
-src = aps.c buf.c bug.c command.c find.c item.c log.c main.c match.c player.c queue.c response.c split.c
+src = aps.c arg.c buf.c bug.c command.c find.c item.c log.c main.c match.c player.c queue.c response.c split.c
obj = ${src:%.c=%.o}
lib = ap
mk = ../mk
diff --git a/aps/player.c b/aps/player.c
@@ -25,6 +25,7 @@
#include <string.h>
#include <unistd.h>
+#include "arg.h"
#include "player.h"
int
@@ -144,3 +145,33 @@ pupdate(struct player *p, int status)
p->state = SUSPENDED;
}
}
+
+void
+pfree(struct player *p)
+{
+ if (p == NULL)
+ return;
+ if (p->state != STOPPED)
+ pstop(p);
+ argfree(p->argv);
+ free(p->path);
+ free(p->prog);
+ free(p);
+}
+
+struct player *
+pnew(char *prog, char **argv)
+{
+ struct player *p;
+
+ p = calloc(1, sizeof(*p));
+ if (p == NULL)
+ return p;
+
+ if ((p->prog = strdup(prog)) == NULL ||
+ (p->argv = argdup(argv, arglen(argv))) == NULL) {
+ pfree(p);
+ return NULL;
+ }
+ return p;
+}
diff --git a/aps/player.h b/aps/player.h
@@ -21,3 +21,5 @@ int pcontinue(struct player *);
int pplay(struct player *, char *);
int ptoggle(struct player *);
void pupdate(struct player *, int);
+void pfree(struct player *);
+struct player *pnew(char *, char **);
diff --git a/aps/queue.c b/aps/queue.c
@@ -28,8 +28,8 @@ int
queue_set(struct aps *aps, struct item *item)
{
aps->queue = item;
- if ((aps->player.state != STOPPED && aps->queue) &&
- (pstop(&aps->player) || pplay(&aps->player, aps->queue->path)))
+ if ((aps->player->state != STOPPED && aps->queue) &&
+ (pstop(aps->player) || pplay(aps->player, aps->queue->path)))
return 1;
return 0;
}
diff --git a/lib/ap/con.c b/lib/ap/con.c
@@ -64,6 +64,8 @@ apopen(char *path, int (*init)(int, const struct sockaddr *, socklen_t), int fla
void
apclose(struct apcon *apc)
{
+ if (apc == NULL)
+ return;
sclose(apc->sock);
if (apc->init == bind)
unlink(apc->path);
