Re: [PATCH] Implement backend program convention command for vhost-user-blk

[Stefan Hajnoczi]

On Fri, Nov 15, 2019 at 02:09:25PM +0800, Micky Yun Chan wrote:

Thanks for the patch!

> +    context = g_option_context_new(NULL);

Missing g_option_context_free() later in this function.  g_auto() is the
easiest way to ensure that.

> +    g_option_context_add_main_entries(context, entries, NULL);
> +    if (!g_option_context_parse(context, &argc, &argv, &error)) {
> +        g_printerr("Option parsing failed: %s\n", error->message);
> +        exit(EXIT_FAILURE);
> +    }
> +    if (opt_print_caps) {
> +        g_print("{\n");
> +        g_print("  \"type\": \"blk\",\n");
> +        g_print("  \"features\": [\n");
> +        g_print("    \"blk-file\",\n");
> +        g_print("    \"read-only\"\n");
> +        g_print("  ]\n");
> +        g_print("}\n");
> +        exit(EXIT_SUCCESS);
>      }
>  
> -    if (!unix_socket || !blk_file) {
> +    if (!opt_blk_file) {
>          printf("Usage: %s [ -b block device or file, -s UNIX domain socket"
>                 " | -r Enable read-only ] | [ -h ]\n", argv[0]);

g_option_context_get_help() can be used instead.  That way the help text
is consistent.

>          return -1;

A good opportunity to switch to exit(EXIT_FAILURE) since main() should
not return -1.

>      }
>  
> -    lsock = unix_sock_new(unix_socket);
> -    if (lsock < 0) {
> -        goto err;
> +    if (opt_socket_path) {
> +        lsock = unix_sock_new(opt_socket_path);
> +        if (lsock < 0) {
> +           exit(EXIT_FAILURE);
> +        }
> +    } else {

  } else if (opt_fd < 0) {
      ...print usage because neither --socket-path nor --fd were given...
  }

signature.asc
Description: PGP signature