Re: [PATCH vdagent-linux] udscs: simplify traffic logging

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>

Acked-by: Victor Toso <victortoso@xxxxxxxxxx>

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@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux Virtualization]     [Linux Virtualization]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]