[Spice-devel] [PATCH vdagent v2 4/8] vdagent: Use glib's commandline parser
Jakub Janků
janku.jakub.jj at gmail.com
Wed Oct 4 22:04:37 UTC 2017
On Wed, Oct 4, 2017 at 11:43 AM, Frediano Ziglio <fziglio at redhat.com> wrote:
>>
>> From: Victor Toso <me at victortoso.com>
>>
>> As we already depend on glib, let's remove code that glib can take
>> care of. In this case, we don't need to handle commandline parsing
>> ourselves.
>>
>> In regard the global variables:
>>
>> * static const char * -> static gchar * [only paths]
>> path variables: portdev, fx_dir, vdagentd_socket
>>
>> * static int -> static gboolean
>> flags: debug, do_daemonize, x11_sync
>>
>> Signed-off-by: Victor Toso <victortoso at redhat.com>
>> ---
>> src/vdagent/vdagent.c | 119
>> ++++++++++++++++++++++++++------------------------
>> 1 file changed, 61 insertions(+), 58 deletions(-)
>>
>> diff --git a/src/vdagent/vdagent.c b/src/vdagent/vdagent.c
>> index 7a2a220..0bb631f 100644
>> --- a/src/vdagent/vdagent.c
>> +++ b/src/vdagent/vdagent.c
>> @@ -44,17 +44,46 @@
>> #include "x11.h"
>> #include "file-xfers.h"
>>
>> -static const char *portdev = DEFAULT_VIRTIO_PORT_PATH;
>> -static const char *vdagentd_socket = VDAGENTD_SOCKET;
>> -static int debug = 0;
>> -static const char *fx_dir = NULL;
>> -static int fx_open_dir = -1;
>> static struct vdagent_x11 *x11 = NULL;
>> static struct vdagent_file_xfers *vdagent_file_xfers = NULL;
>> static struct udscs_connection *client = NULL;
>> static int quit = 0;
>> static int version_mismatch = 0;
>>
>> +/* Command line options */
>> +static gboolean debug = FALSE;
>> +static gboolean x11_sync = FALSE;
>> +static gboolean do_daemonize = TRUE;
>> +static gint fx_open_dir = -1;
>> +static gchar *fx_dir = NULL;
>> +static gchar *portdev = NULL;
>> +static gchar *vdagentd_socket = NULL;
>> +
>> +static GOptionEntry entries[] = {
>> + { "debug", 'd', 0,
>> + G_OPTION_ARG_NONE, &debug,
>> + "Enable debug", NULL },
>> + { "virtio-serial-port-path", 's', 0,
>> + G_OPTION_ARG_STRING, &portdev,
>> + "Set virtio-serial path (" DEFAULT_VIRTIO_PORT_PATH ")", NULL },
>> + { "vdagentd-socket", 'S', 0, G_OPTION_ARG_STRING,
>> + &vdagentd_socket,
>> + "Set spice-vdagentd socket (" VDAGENTD_SOCKET ")", NULL },
>> + { "foreground", 'x', G_OPTION_FLAG_REVERSE,
>> + G_OPTION_ARG_NONE, &do_daemonize,
>> + "Do not daemonize the agent", NULL },
>> + { "file-xfer-save-dir", 'f', 0,
>> + G_OPTION_ARG_STRING, &fx_dir,
>> + "Set directory to file transfers files",
>> "<dir|xdg-desktop|xdg-download>"},
>> + { "file-xfer-open-dir", 'o', 0,
>> + G_OPTION_ARG_INT, &fx_open_dir,
>> + "Open directory after completing file transfer", "<0|1>" },
>> + { "x11-abort-on-error", 'y', 0,
>> + G_OPTION_ARG_NONE, &x11_sync,
>> + "Aborts on errors from X11", NULL },
>> + { NULL }
>> +};
>> +
>> /**
>> * xfer_get_download_directory
>> *
>> @@ -215,22 +244,6 @@ static int client_setup(int reconnect)
>> return client == NULL;
>> }
>>
>> -static void usage(FILE *fp)
>> -{
>> - fprintf(fp,
>> - "Usage: spice-vdagent [OPTIONS]\n\n"
>> - "Spice guest agent X11 session agent, version %s.\n\n"
>> - "Options:\n"
>> - " -h print this text\n"
>> - " -d log debug messages\n"
>> - " -s <port> set virtio serial port\n"
>> - " -S <filename> set udcs socket\n"
>> - " -x don't daemonize\n"
>> - " -f <dir|xdg-desktop|xdg-download> file xfer save dir\n"
>> - " -o <0|1> open dir on file xfer
>> completion\n",
>
> Note that -y was not documented now is it.
> Was this intentional, kind of a developer option not supposed
> to be maintained?
I've looked at the commit message and it doesn't say,
why this option isn't mentioned in the help output.
I'll add G_OPTION_FLAG_HIDDEN to this option entry
to copy the current behavior.
>
>> - VERSION);
>> -}
>> -
>> static void quit_handler(int sig)
>> {
>> quit = 1;
>> @@ -294,47 +307,33 @@ static int file_test(const char *path)
>> int main(int argc, char *argv[])
>> {
>> fd_set readfds, writefds;
>> - int c, n, nfds, x11_fd;
>> - int do_daemonize = 1;
>> + int n, nfds, x11_fd;
>> int parent_socket = 0;
>> - int x11_sync = 0;
>> struct sigaction act;
>> -
>> - for (;;) {
>> - if (-1 == (c = getopt(argc, argv, "-dxhys:f:o:S:")))
>> - break;
>> - switch (c) {
>> - case 'd':
>> - debug++;
>
> Here debug was supporting multiple debugging level but in the agent
> this feature is not used so no regression are created.
>
>> - break;
>> - case 's':
>> - portdev = optarg;
>> - break;
>> - case 'x':
>> - do_daemonize = 0;
>> - break;
>> - case 'y':
>> - x11_sync = 1;
>> - break;
>> - case 'h':
>> - usage(stdout);
>> - return 0;
>> - case 'f':
>> - fx_dir = optarg;
>> - break;
>> - case 'o':
>> - fx_open_dir = atoi(optarg);
>> - break;
>> - case 'S':
>> - vdagentd_socket = optarg;
>> - break;
>> - default:
>> - fputs("\n", stderr);
>> - usage(stderr);
>> - return 1;
>> - }
>> + GOptionContext *context;
>> + GError *error = NULL;
>> +
>> + context = g_option_context_new(NULL);
>> + g_option_context_add_main_entries(context, entries, NULL);
>> + g_option_context_set_summary(context,
>> + "\tSpice session guest agent: X11\n"
>> + "\tVersion: " VERSION);
>> + g_option_context_parse(context, &argc, &argv, &error);
>> + g_option_context_free(context);
>> +
>> + if (error != NULL) {
>> + g_printerr("Invalid arguments, %s\n", error->message);
>> + g_clear_error(&error);
>> + return -1;
>> }
>>
>> + /* Set default path value if none was set */
>> + if (portdev == NULL)
>> + portdev = g_strdup(DEFAULT_VIRTIO_PORT_PATH);
>> +
>> + if (vdagentd_socket == NULL)
>> + vdagentd_socket = g_strdup(VDAGENTD_SOCKET);
>> +
>> memset(&act, 0, sizeof(act));
>> act.sa_flags = SA_RESTART;
>> act.sa_handler = quit_handler;
>> @@ -410,5 +409,9 @@ reconnect:
>> if (!quit && do_daemonize)
>> goto reconnect;
>>
>> + g_free(fx_dir);
>> + g_free(portdev);
>> + g_free(vdagentd_socket);
>> +
>> return 0;
>> }
>
> otherwise,
>
> Acked-by: Frediano Ziglio <fziglio at redhat.com>
>
> Frediano
Thanks,
Jakub
More information about the Spice-devel
mailing list