[Spice-devel] [PATCH vdagent-linux] udscs: simplify traffic logging
Victor Toso
victortoso at redhat.com
Fri Sep 21 19:38:21 UTC 2018
Hi,
On Fri, Sep 21, 2018 at 05:51:45PM +0200, Jakub Janků wrote:
> Remove @type_to_string, @no_types arguments from
> udscs_connect(), udscs_create_server{,_for_fd}().
> udscs is used only in vdagent.c and vdagentd.c
> and in both cases the args are the same
> (vdagentd_messages, VDAGENTD_NO_MESSAGES).
>
> Add debug_print_message_header().
>
> Signed-off-by: Jakub Janků <jjanku at redhat.com>
Acked-by: Victor Toso <victortoso at redhat.com>
And pushed:
https://gitlab.freedesktop.org/spice/linux/vd_agent/commit/b245cbdd59a5555fb2ff6f7dd7febb6a14b21db5
Thanks!
> ---
> src/udscs.c | 59 ++++++++++++++++-------------------------
> src/udscs.h | 16 ++++-------
> src/vdagent/vdagent.c | 3 +--
> src/vdagentd/vdagentd.c | 5 +---
> 4 files changed, 30 insertions(+), 53 deletions(-)
>
> diff --git a/src/udscs.c b/src/udscs.c
> index 59e24d8..62abc97 100644
> --- a/src/udscs.c
> +++ b/src/udscs.c
> @@ -34,6 +34,7 @@
> #include <glib.h>
> #include <glib-unix.h>
> #include "udscs.h"
> +#include "vdagentd-proto-strings.h"
>
> struct udscs_buf {
> uint8_t *buf;
> @@ -45,8 +46,6 @@ struct udscs_buf {
>
> struct udscs_connection {
> int fd;
> - const char * const *type_to_string;
> - int no_types;
> int debug;
> void *user_data;
> #ifndef UDSCS_NO_SERVER
> @@ -78,18 +77,32 @@ static gboolean udscs_io_channel_cb(GIOChannel *source,
> GIOCondition condition,
> gpointer data);
>
> +static void debug_print_message_header(struct udscs_connection *conn,
> + struct udscs_message_header *header,
> + const gchar *direction)
> +{
> + const gchar *type = "invalid message";
> +
> + if (conn == NULL || conn->debug == FALSE)
> + return;
> +
> + if (header->type < G_N_ELEMENTS(vdagentd_messages))
> + type = vdagentd_messages[header->type];
> +
> + syslog(LOG_DEBUG, "%p %s %s, arg1: %u, arg2: %u, size %u",
> + conn, direction, type, header->arg1, header->arg2, header->size);
> +}
> +
> 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 debug)
> {
> int c;
> struct sockaddr_un address;
> struct udscs_connection *conn;
>
> conn = g_new0(struct udscs_connection, 1);
> - conn->type_to_string = type_to_string;
> - conn->no_types = no_types;
> conn->debug = debug;
>
> conn->fd = socket(PF_UNIX, SOCK_STREAM, 0);
> @@ -203,15 +216,7 @@ void udscs_write(struct udscs_connection *conn, uint32_t type, uint32_t arg1,
> memcpy(new_wbuf->buf, &header, sizeof(header));
> memcpy(new_wbuf->buf + sizeof(header), data, size);
>
> - if (conn->debug) {
> - if (type < conn->no_types)
> - syslog(LOG_DEBUG, "%p sent %s, arg1: %u, arg2: %u, size %u",
> - conn, conn->type_to_string[type], arg1, arg2, size);
> - else
> - syslog(LOG_DEBUG,
> - "%p sent invalid message %u, arg1: %u, arg2: %u, size %u",
> - conn, type, arg1, arg2, size);
> - }
> + debug_print_message_header(conn, &header, "sent");
>
> if (conn->io_channel && conn->write_watch_id == 0)
> conn->write_watch_id =
> @@ -238,18 +243,7 @@ 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);
> - }
> + debug_print_message_header(conn, &conn->header, "received");
>
> if (conn->read_callback) {
> conn->read_callback(connp, &conn->header, conn->data.buf);
> @@ -373,8 +367,6 @@ static gboolean udscs_io_channel_cb(GIOChannel *source,
>
> struct udscs_server {
> int fd;
> - const char * const *type_to_string;
> - int no_types;
> int debug;
> struct udscs_connection connections_head;
> udscs_connect_callback connect_callback;
> @@ -386,7 +378,7 @@ 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 debug)
> {
> struct udscs_server *server;
>
> @@ -396,8 +388,6 @@ struct udscs_server *udscs_create_server_for_fd(int fd,
> }
>
> server = g_new0(struct udscs_server, 1);
> - server->type_to_string = type_to_string;
> - server->no_types = no_types;
> server->debug = debug;
> server->fd = fd;
> server->connect_callback = connect_callback;
> @@ -411,7 +401,7 @@ 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)
> + int debug)
> {
> int c;
> int fd;
> @@ -441,8 +431,7 @@ struct udscs_server *udscs_create_server(const char *socketname,
> }
>
> server = udscs_create_server_for_fd(fd, connect_callback, read_callback,
> - disconnect_callback, type_to_string,
> - no_types, debug);
> + disconnect_callback, debug);
>
> if (!server) {
> close(fd);
> @@ -489,8 +478,6 @@ static void udscs_server_accept(struct udscs_server *server) {
>
> new_conn = g_new0(struct udscs_connection, 1);
> 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;
> diff --git a/src/udscs.h b/src/udscs.h
> index a863e16..363ca18 100644
> --- a/src/udscs.h
> +++ b/src/udscs.h
> @@ -61,15 +61,12 @@ typedef void (*udscs_disconnect_callback)(struct udscs_connection *conn);
> * Only sockets bound to a pathname are supported.
> *
> * If debug is true then the events on this connection will be traced.
> - * This includes the incoming and outgoing message names. So when debug is true
> - * no_types must be set to the value of the highest valid message id + 1,
> - * and type_to_string must point to a string array of size no_types for
> - * converting the message ids to their names.
> + * This includes the incoming and outgoing message names.
> */
> 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 debug);
>
> /* Close the connection, releases the corresponding resources and
> * sets *connp to NULL.
> @@ -112,7 +109,7 @@ 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 debug);
>
> /* Create the unix domain socket specified by socketname and
> * start listening on it.
> @@ -120,16 +117,13 @@ struct udscs_server *udscs_create_server_for_fd(int fd,
> *
> * If debug is true then the events on this socket and related individual
> * connections will be traced.
> - * This includes the incoming and outgoing message names. So when debug is true
> - * no_types must be set to the value of the highest valid message id + 1,
> - * and type_to_string must point to a string array of size no_types for
> - * converting the message ids to their names.
> + * This includes the incoming and outgoing message names.
> */
> 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);
> + int debug);
>
> /* Close all the server's connections and releases the corresponding
> * resources.
> diff --git a/src/vdagent/vdagent.c b/src/vdagent/vdagent.c
> index c5e5952..f7c8b72 100644
> --- a/src/vdagent/vdagent.c
> +++ b/src/vdagent/vdagent.c
> @@ -42,7 +42,6 @@
>
> #include "udscs.h"
> #include "vdagentd-proto.h"
> -#include "vdagentd-proto-strings.h"
> #include "audio.h"
> #include "x11.h"
> #include "file-xfers.h"
> @@ -369,7 +368,7 @@ static gboolean vdagent_init_async_cb(gpointer user_data)
>
> agent->conn = udscs_connect(vdagentd_socket,
> daemon_read_complete, daemon_disconnect_cb,
> - vdagentd_messages, VDAGENTD_NO_MESSAGES, debug);
> + debug);
> if (agent->conn == NULL) {
> g_timeout_add_seconds(1, vdagent_init_async_cb, agent);
> return G_SOURCE_REMOVE;
> diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c
> index 6b04813..99683da 100644
> --- a/src/vdagentd/vdagentd.c
> +++ b/src/vdagentd/vdagentd.c
> @@ -42,7 +42,6 @@
>
> #include "udscs.h"
> #include "vdagentd-proto.h"
> -#include "vdagentd-proto-strings.h"
> #include "uinput.h"
> #include "xorg-conf.h"
> #include "virtio-port.h"
> @@ -1139,8 +1138,7 @@ int main(int argc, char *argv[])
> server = udscs_create_server_for_fd(SD_LISTEN_FDS_START, agent_connect,
> agent_read_complete,
> agent_disconnect,
> - vdagentd_messages,
> - VDAGENTD_NO_MESSAGES, debug);
> + debug);
> own_socket = FALSE;
> } else
> /* systemd socket activation not enabled, create our own */
> @@ -1148,7 +1146,6 @@ int main(int argc, char *argv[])
> {
> server = udscs_create_server(vdagentd_socket, agent_connect,
> agent_read_complete, agent_disconnect,
> - vdagentd_messages, VDAGENTD_NO_MESSAGES,
> debug);
> }
>
> --
> 2.17.1
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20180921/e20a0308/attachment.sig>
More information about the Spice-devel
mailing list