Re: [PATCH vdagent v2 4/8] vdagent: Use glib's commandline parser

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

 



On Wed, Oct 4, 2017 at 11:43 AM, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote:
>>
>> From: Victor Toso <me@xxxxxxxxxxxxxx>
>>
>> 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@xxxxxxxxxx>
>> ---
>>  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@xxxxxxxxxx>
>
> Frediano

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




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