Rewrite udscs.c using VDAgentConnection to integrate it
into GMainLoop and simplify the code.
udscs_destroy_connection() does NOT close
the underlying FD immediately.
Apart from that, the behavior stays the same.
Drop support for select() in udscs_server, remove
udscs_server_fill_fds(), udscs_server_handle_fds().
---
src/udscs.c | 515 ++++++++++++----------------------------
src/udscs.h | 15 --
src/vdagentd/vdagentd.c | 7 +-
3 files changed, 150 insertions(+), 387 deletions(-)
diff --git a/src/udscs.c b/src/udscs.c
index a77da99..45565b6 100644
--- a/src/udscs.c
+++ b/src/udscs.c
@@ -24,43 +24,21 @@
#include <config.h>
#endif
-#include <stdio.h>
#include <stdlib.h>
#include <syslog.h>
-#include <unistd.h>
-#include <errno.h>
-#include <sys/socket.h>
-#include <sys/un.h>
-#include <glib.h>
#include <glib-unix.h>
-#include "udscs.h"
-
-struct udscs_buf {
- uint8_t *buf;
- size_t pos;
- size_t size;
+#include <gio/gunixsocketaddress.h>
- struct udscs_buf *next;
-};
+#include "udscs.h"
+#include "vdagent-connection.h"
struct udscs_connection {
- int fd;
const char * const *type_to_string;
int no_types;
int debug;
void *user_data;
-#ifndef UDSCS_NO_SERVER
- struct ucred peer_cred;
-#endif
- /* Read stuff, single buffer, separate header and data buffer */
- int header_read;
- struct udscs_message_header header;
- struct udscs_buf data;
-
- /* Writes are stored in a linked list of buffers, with both the header
- + data for a single message in 1 buffer. */
- struct udscs_buf *write_buf;
+ VDAgentConnection *conn;
/* Callbacks */
udscs_read_callback read_callback;
@@ -68,25 +46,61 @@ struct udscs_connection {
struct udscs_connection *next;
struct udscs_connection *prev;
-
- GIOChannel *io_channel;
- guint write_watch_id;
- guint read_watch_id;
};
-static gboolean udscs_io_channel_cb(GIOChannel *source,
- GIOCondition condition,
- gpointer data);
+static gboolean conn_header_read_cb(gpointer header_buff,
+ gsize *body_size,
+ gpointer user_data)
+{
+ struct udscs_message_header *header = header_buff;
+ *body_size = header->size;
+ return TRUE;
+}
+
+static gboolean conn_read_cb(gpointer header_buff,
+ gpointer data,
+ gpointer user_data)
+{
+ struct udscs_connection *conn = user_data;
+ struct udscs_message_header *header = header_buff;
+
+ if (conn->debug) {
+ if (header->type < conn->no_types)
+ syslog(LOG_DEBUG,
+ "%p received %s, arg1: %u, arg2: %u, size %u",
+ conn, conn->type_to_string[header->type],
+ header->arg1, header->arg2, header->size);
+ else
+ syslog(LOG_DEBUG,
+ "%p received invalid message %u, arg1: %u, arg2: %u, size %u",
+ conn, header->type, header->arg1, header->arg2,
+ header->size);
+ }
+
+ if (conn->read_callback) {
+ conn->read_callback(&conn, header, data);
+ }
+ return conn != NULL;
+}
+
+static void conn_error_cb(gpointer user_data)
+{
+ struct udscs_connection *conn = user_data;
+ udscs_destroy_connection(&conn);
+}
struct udscs_connection *udscs_connect(const char *socketname,
udscs_read_callback read_callback,
udscs_disconnect_callback disconnect_callback,
const char * const type_to_string[], int no_types, int debug)
{
- int c;
- struct sockaddr_un address;
+ GIOStream *io_stream;
struct udscs_connection *conn;
+ io_stream = vdagent_socket_connect(socketname);
+ if (io_stream == NULL)
+ return NULL;
+
conn = calloc(1, sizeof(*conn));
if (!conn)
return NULL;
@@ -94,36 +108,13 @@ struct udscs_connection *udscs_connect(const char *socketname,
conn->type_to_string = type_to_string;
conn->no_types = no_types;
conn->debug = debug;
-
- conn->fd = socket(PF_UNIX, SOCK_STREAM, 0);
- if (conn->fd == -1) {
- syslog(LOG_ERR, "creating unix domain socket: %m");
- free(conn);
- return NULL;
- }
-
- address.sun_family = AF_UNIX;
- snprintf(address.sun_path, sizeof(address.sun_path), "%s", socketname);
- c = connect(conn->fd, (struct sockaddr *)&address, sizeof(address));
- if (c != 0) {
- if (conn->debug) {
- syslog(LOG_DEBUG, "connect %s: %m", socketname);
- }
- free(conn);
- return NULL;
- }
-
- conn->io_channel = g_io_channel_unix_new(conn->fd);
- if (!conn->io_channel) {
- udscs_destroy_connection(&conn);
- return NULL;
- }
- conn->read_watch_id =
- g_io_add_watch(conn->io_channel,
- G_IO_IN | G_IO_ERR | G_IO_NVAL,
- udscs_io_channel_cb,
- conn);
-
+ conn->conn = vdagent_connection_new(io_stream,
+ FALSE,
+ sizeof(struct udscs_message_header),
+ conn_header_read_cb,
+ conn_read_cb,
+ conn_error_cb,
+ conn);
conn->read_callback = read_callback;
conn->disconnect_callback = disconnect_callback;
@@ -135,7 +126,6 @@ struct udscs_connection *udscs_connect(const char *socketname,
void udscs_destroy_connection(struct udscs_connection **connp)
{
- struct udscs_buf *wbuf, *next_wbuf;
struct udscs_connection *conn = *connp;
if (!conn)
@@ -144,29 +134,12 @@ void udscs_destroy_connection(struct udscs_connection **connp)
if (conn->disconnect_callback)
conn->disconnect_callback(conn);
- wbuf = conn->write_buf;
- while (wbuf) {
- next_wbuf = wbuf->next;
- free(wbuf->buf);
- free(wbuf);
- wbuf = next_wbuf;
- }
-
- free(conn->data.buf);
- conn->data.buf = NULL;
-
if (conn->next)
conn->next->prev = conn->prev;
if (conn->prev)
conn->prev->next = conn->next;
- close(conn->fd);
-
- if (conn->write_watch_id != 0)
- g_source_remove(conn->write_watch_id);
- if (conn->read_watch_id != 0)
- g_source_remove(conn->read_watch_id);
- g_clear_pointer(&conn->io_channel, g_io_channel_unref);
+ vdagent_connection_destroy(conn->conn);
if (conn->debug)
syslog(LOG_DEBUG, "%p disconnected", conn);
@@ -191,29 +164,22 @@ void *udscs_get_user_data(struct udscs_connection *conn)
int udscs_write(struct udscs_connection *conn, uint32_t type, uint32_t arg1,
uint32_t arg2, const uint8_t *data, uint32_t size)
{
- struct udscs_buf *wbuf, *new_wbuf;
+ guchar *buff;
+ guint buff_size;
struct udscs_message_header header;
- new_wbuf = malloc(sizeof(*new_wbuf));
- if (!new_wbuf)
+ buff_size = sizeof(header) + size;
+ buff = malloc(buff_size);
+ if (buff == NULL)
return -1;
- new_wbuf->pos = 0;
- new_wbuf->size = sizeof(header) + size;
- new_wbuf->next = NULL;
- new_wbuf->buf = malloc(new_wbuf->size);
- if (!new_wbuf->buf) {
- free(new_wbuf);
- return -1;
- }
-
header.type = type;
header.arg1 = arg1;
header.arg2 = arg2;
header.size = size;
- memcpy(new_wbuf->buf, &header, sizeof(header));
- memcpy(new_wbuf->buf + sizeof(header), data, size);
+ memcpy(buff, &header, sizeof(header));
+ memcpy(buff + sizeof(header), data, size);
if (conn->debug) {
if (type < conn->no_types)
@@ -225,173 +191,17 @@ int udscs_write(struct udscs_connection *conn, uint32_t type, uint32_t arg1,
conn, type, arg1, arg2, size);
}
- if (conn->io_channel && conn->write_watch_id == 0)
- conn->write_watch_id =
- g_io_add_watch(conn->io_channel,
- G_IO_OUT | G_IO_ERR | G_IO_NVAL,
- udscs_io_channel_cb,
- conn);
-
- if (!conn->write_buf) {
- conn->write_buf = new_wbuf;
- return 0;
- }
-
- /* maybe we should limit the write_buf stack depth ? */
- wbuf = conn->write_buf;
- while (wbuf->next)
- wbuf = wbuf->next;
-
- wbuf->next = new_wbuf;
-
+ vdagent_connection_write(conn->conn, buff, buff_size);
return 0;
}
-/* A helper for udscs_do_read() */
-static void udscs_read_complete(struct udscs_connection **connp)
-{
- struct udscs_connection *conn = *connp;
-
- if (conn->debug) {
- if (conn->header.type < conn->no_types)
- syslog(LOG_DEBUG,
- "%p received %s, arg1: %u, arg2: %u, size %u",
- conn, conn->type_to_string[conn->header.type],
- conn->header.arg1, conn->header.arg2, conn->header.size);
- else
- syslog(LOG_DEBUG,
- "%p received invalid message %u, arg1: %u, arg2: %u, size %u",
- conn, conn->header.type, conn->header.arg1, conn->header.arg2,
- conn->header.size);
- }
-
- if (conn->read_callback) {
- conn->read_callback(connp, &conn->header, conn->data.buf);
- if (!*connp) /* Was the connection disconnected by the callback ? */
- return;
- }
-
- free(conn->data.buf);
- memset(&conn->data, 0, sizeof(conn->data)); /* data.buf = NULL */
- conn->header_read = 0;
-}
-
-static void udscs_do_read(struct udscs_connection **connp)
-{
- ssize_t n;
- size_t to_read;
- uint8_t *dest;
- struct udscs_connection *conn = *connp;
-
- if (conn->header_read < sizeof(conn->header)) {
- to_read = sizeof(conn->header) - conn->header_read;
- dest = (uint8_t *)&conn->header + conn->header_read;
- } else {
- to_read = conn->data.size - conn->data.pos;
- dest = conn->data.buf + conn->data.pos;
- }
-
- n = read(conn->fd, dest, to_read);
- if (n < 0) {
- if (errno == EINTR)
- return;
- syslog(LOG_ERR, "reading unix domain socket: %m, disconnecting %p",
- conn);
- }
- if (n <= 0) {
- udscs_destroy_connection(connp);
- return;
- }
-
- if (conn->header_read < sizeof(conn->header)) {
- conn->header_read += n;
- if (conn->header_read == sizeof(conn->header)) {
- if (conn->header.size == 0) {
- udscs_read_complete(connp);
- return;
- }
- conn->data.pos = 0;
- conn->data.size = conn->header.size;
- conn->data.buf = malloc(conn->data.size);
- if (!conn->data.buf) {
- syslog(LOG_ERR, "out of memory, disconnecting %p", conn);
- udscs_destroy_connection(connp);
- return;
- }
- }
- } else {
- conn->data.pos += n;
- if (conn->data.pos == conn->data.size)
- udscs_read_complete(connp);
- }
-}
-
-static void udscs_do_write(struct udscs_connection **connp)
-{
- ssize_t n;
- size_t to_write;
- struct udscs_connection *conn = *connp;
-
- struct udscs_buf* wbuf = conn->write_buf;
- if (!wbuf) {
- syslog(LOG_ERR,
- "%p do_write called on a connection without a write buf ?!",
- conn);
- return;
- }
-
- to_write = wbuf->size - wbuf->pos;
- n = write(conn->fd, wbuf->buf + wbuf->pos, to_write);
- if (n < 0) {
- if (errno == EINTR)
- return;
- syslog(LOG_ERR, "writing to unix domain socket: %m, disconnecting %p",
- conn);
- udscs_destroy_connection(connp);
- return;
- }
-
- wbuf->pos += n;
- if (wbuf->pos == wbuf->size) {
- conn->write_buf = wbuf->next;
- free(wbuf->buf);
- free(wbuf);
- }
-}
-
-static gboolean udscs_io_channel_cb(GIOChannel *source,
- GIOCondition condition,
- gpointer data)
-{
- struct udscs_connection *conn = data;
-
- if (condition & G_IO_IN) {
- udscs_do_read(&conn);
- if (conn == NULL)
- return G_SOURCE_REMOVE;
- return G_SOURCE_CONTINUE;
- }
- if (condition & G_IO_OUT) {
- udscs_do_write(&conn);
- if (conn == NULL)
- return G_SOURCE_REMOVE;
- if (conn->write_buf)
- return G_SOURCE_CONTINUE;
- conn->write_watch_id = 0;
- return G_SOURCE_REMOVE;
- }
-
- udscs_destroy_connection(&conn);
- return G_SOURCE_REMOVE;
-}
-
-
#ifndef UDSCS_NO_SERVER
/* ---------- Server-side implementation ---------- */
struct udscs_server {
- int fd;
+ GSocketService *service;
+
const char * const *type_to_string;
int no_types;
int debug;
@@ -401,7 +211,12 @@ struct udscs_server {
udscs_disconnect_callback disconnect_callback;
};
-struct udscs_server *udscs_create_server_for_fd(int fd,
+static gboolean udscs_server_accept_cb(GSocketService *service,
+ GSocketConnection *socket_conn,
+ GObject *source_object,
+ gpointer user_data);
+
+static struct udscs_server *udscs_server_new(
udscs_connect_callback connect_callback,
udscs_read_callback read_callback,
udscs_disconnect_callback disconnect_callback,
@@ -409,11 +224,6 @@ struct udscs_server *udscs_create_server_for_fd(int fd,
{
struct udscs_server *server;
- if (fd <= 0) {
- syslog(LOG_ERR, "Invalid file descriptor: %i", fd);
- return NULL;
- }
-
server = calloc(1, sizeof(*server));
if (!server)
return NULL;
@@ -421,53 +231,76 @@ struct udscs_server *udscs_create_server_for_fd(int fd,
server->type_to_string = type_to_string;
server->no_types = no_types;
server->debug = debug;
- server->fd = fd;
server->connect_callback = connect_callback;
server->read_callback = read_callback;
server->disconnect_callback = disconnect_callback;
+ server->service = g_socket_service_new();
+
+ g_signal_connect(server->service, "incoming",
+ G_CALLBACK(udscs_server_accept_cb), server);
return server;
}
-struct udscs_server *udscs_create_server(const char *socketname,
+struct udscs_server *udscs_create_server_for_fd(int fd,
udscs_connect_callback connect_callback,
udscs_read_callback read_callback,
udscs_disconnect_callback disconnect_callback,
const char * const type_to_string[], int no_types, int debug)
{
- int c;
- int fd;
- struct sockaddr_un address;
struct udscs_server *server;
+ GSocket *socket;
+ GError *err = NULL;
- fd = socket(PF_UNIX, SOCK_STREAM, 0);
- if (fd == -1) {
- syslog(LOG_ERR, "creating unix domain socket: %m");
+ server = udscs_server_new(connect_callback, read_callback, disconnect_callback,
+ type_to_string, no_types, debug);
+ if (server == NULL)
return NULL;
- }
- address.sun_family = AF_UNIX;
- snprintf(address.sun_path, sizeof(address.sun_path), "%s", socketname);
- c = bind(fd, (struct sockaddr *)&address, sizeof(address));
- if (c != 0) {
- syslog(LOG_ERR, "bind %s: %m", socketname);
- close(fd);
- return NULL;
- }
+ socket = g_socket_new_from_fd(fd, &err);
+ if (err)
+ goto error;
+ g_socket_listener_add_socket(G_SOCKET_LISTENER(server->service),
+ socket, NULL, &err);
+ g_object_unref(socket);
+ if (err)
+ goto error;
- c = listen(fd, 5);
- if (c != 0) {
- syslog(LOG_ERR, "listen: %m");
- close(fd);
- return NULL;
- }
+ return server;
+error:
+ syslog(LOG_ERR, "%s: %s", __func__, err->message);
+ g_error_free(err);
+ udscs_destroy_server(server);
+ return NULL;
+}
- server = udscs_create_server_for_fd(fd, connect_callback, read_callback,
- disconnect_callback, type_to_string,
- no_types, debug);
+struct udscs_server *udscs_create_server(const char *socketname,
+ udscs_connect_callback connect_callback,
+ udscs_read_callback read_callback,
+ udscs_disconnect_callback disconnect_callback,
+ const char * const type_to_string[], int no_types, int debug)
+{
+ struct udscs_server *server;
+ GSocketAddress *socket_addr;
+ GError *err = NULL;
+
+ server = udscs_server_new(connect_callback, read_callback, disconnect_callback,
+ type_to_string, no_types, debug);
+ if (server == NULL)
+ return NULL;
- if (!server) {
- close(fd);
+ socket_addr = g_unix_socket_address_new(socketname);
+ g_socket_listener_add_address(G_SOCKET_LISTENER(server->service),
+ socket_addr,
+ G_SOCKET_TYPE_STREAM,
+ G_SOCKET_PROTOCOL_DEFAULT,
+ NULL, NULL, &err);
+ g_object_unref(socket_addr);
+ if (err) {
+ syslog(LOG_ERR, "%s: %s", __func__, err->message);
+ g_error_free(err);
+ udscs_destroy_server(server);
+ return NULL;
}
return server;
@@ -486,50 +319,49 @@ void udscs_destroy_server(struct udscs_server *server)
udscs_destroy_connection(&conn);
conn = next_conn;
}
- close(server->fd);
+ g_object_unref(server->service);
free(server);
}
int udscs_get_peer_pid(struct udscs_connection *conn)
{
- return (int)conn->peer_cred.pid;
+ GCredentials *cred = vdagent_connection_get_peer_credentials(conn->conn);
+ return cred ? g_credentials_get_unix_pid(cred, NULL) : -1;
}
-static void udscs_server_accept(struct udscs_server *server) {
+static gboolean udscs_server_accept_cb(GSocketService *service,
+ GSocketConnection *socket_conn,
+ GObject *source_object,
+ gpointer user_data)
+{
+ struct udscs_server *server = user_data;
struct udscs_connection *new_conn, *conn;
- struct sockaddr_un address;
- socklen_t length = sizeof(address);
- int r, fd;
-
- fd = accept(server->fd, (struct sockaddr *)&address, &length);
- if (fd == -1) {
- if (errno == EINTR)
- return;
- syslog(LOG_ERR, "accept: %m");
- return;
- }
new_conn = calloc(1, sizeof(*conn));
if (!new_conn) {
syslog(LOG_ERR, "out of memory, disconnecting new client");
- close(fd);
- return;
+ return TRUE;
}
- new_conn->fd = fd;
new_conn->type_to_string = server->type_to_string;
new_conn->no_types = server->no_types;
new_conn->debug = server->debug;
new_conn->read_callback = server->read_callback;
new_conn->disconnect_callback = server->disconnect_callback;
- length = sizeof(new_conn->peer_cred);
- r = getsockopt(fd, SOL_SOCKET, SO_PEERCRED, &new_conn->peer_cred, &length);
- if (r != 0) {
- syslog(LOG_ERR, "Could not get peercred, disconnecting new client");
- close(fd);
- free(new_conn);
- return;
+ g_object_ref(socket_conn);
+ new_conn->conn = vdagent_connection_new(G_IO_STREAM(socket_conn),
+ FALSE,
+ sizeof(struct udscs_message_header),
+ conn_header_read_cb,
+ conn_read_cb,
+ conn_error_cb,
+ new_conn);
+
+ if (udscs_get_peer_pid(new_conn) == -1) {
+ syslog(LOG_ERR, "Could not get peer PID, disconnecting new client");
+ vdagent_connection_destroy(new_conn->conn);
+ return TRUE;
}
conn = &server->connections_head;
@@ -545,59 +377,8 @@ static void udscs_server_accept(struct udscs_server *server) {
if (server->connect_callback)
server->connect_callback(new_conn);
-}
-
-int udscs_server_fill_fds(struct udscs_server *server, fd_set *readfds,
- fd_set *writefds)
-{
- struct udscs_connection *conn;
- int nfds;
-
- if (!server)
- return -1;
-
- nfds = server->fd + 1;
- FD_SET(server->fd, readfds);
-
- conn = server->connections_head.next;
- while (conn) {
- FD_SET(conn->fd, readfds);
- if (conn->write_buf)
- FD_SET(conn->fd, writefds);
- if (conn->fd >= nfds)
- nfds = conn->fd + 1;
-
- conn = conn->next;
- }
-
- return nfds;
-}
-
-void udscs_server_handle_fds(struct udscs_server *server, fd_set *readfds,
- fd_set *writefds)
-{
- struct udscs_connection *conn, *next_conn;
-
- if (!server)
- return;
-
- if (FD_ISSET(server->fd, readfds))
- udscs_server_accept(server);
-
- conn = server->connections_head.next;
- while (conn) {
- /* conn may be destroyed by udscs_do_read() or udscs_do_write()
- * (when disconnected), so get the next connection first. */
- next_conn = conn->next;
-
- if (FD_ISSET(conn->fd, readfds))
- udscs_do_read(&conn);
- if (conn && FD_ISSET(conn->fd, writefds))
- udscs_do_write(&conn);
-
- conn = next_conn;
- }
+ return TRUE;
}
int udscs_server_write_all(struct udscs_server *server,
diff --git a/src/udscs.h b/src/udscs.h
index 4f47b7f..dba3fb9 100644
--- a/src/udscs.h
+++ b/src/udscs.h
@@ -22,9 +22,7 @@
#ifndef __UDSCS_H
#define __UDSCS_H
-#include <stdio.h>
#include <stdint.h>
-#include <sys/select.h>
#include <sys/socket.h>
@@ -158,19 +156,6 @@ typedef int (*udscs_for_all_clients_callback)(struct udscs_connection **connp,
int udscs_server_for_all_clients(struct udscs_server *server,
udscs_for_all_clients_callback func, void *priv);
-/* Given a udscs server, fill the fd_sets pointed to by readfds and
- * writefds for select() usage.
- * Return value: value of the highest fd + 1 or -1 if server is NULL
- */
-int udscs_server_fill_fds(struct udscs_server *server, fd_set *readfds,
- fd_set *writefds);
-
-/* Handle any events flagged by select for the given udscs server.
- * Does nothing if server is NULL.
- */
-void udscs_server_handle_fds(struct udscs_server *server, fd_set *readfds,
- fd_set *writefds);
-
/* Returns the peer's PID. */
int udscs_get_peer_pid(struct udscs_connection *conn);
diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c
index 53d5516..027406f 100644
--- a/src/vdagentd/vdagentd.c
+++ b/src/vdagentd/vdagentd.c
@@ -1130,13 +1130,10 @@ int main(int argc, char *argv[])
}
if (!server) {
- if (errno == EADDRINUSE) {
- syslog(LOG_CRIT, "Fatal the server socket %s exists already. Delete it?",
- vdagentd_socket);
- } else if (errno == ENOMEM) {
+ if (errno == ENOMEM) {
syslog(LOG_CRIT, "Fatal could not allocate memory for udscs server");
} else {
- syslog(LOG_CRIT, "Fatal could not create the server socket %s: %m",
+ syslog(LOG_CRIT, "Fatal could not create the server socket %s",
vdagentd_socket);
}
return 1;
--
2.17.1
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel