Re: [Qemu-devel] [PATCH v2 04/42] ivshmem-server: Fix and clean up command line help

[Marc-André Lureau]

On Mon, Mar 7, 2016 at 8:25 PM, Markus Armbruster <address@hidden> wrote:
> Burying error messages in ~20 lines of usage help is bad form.  Print
> a single line pointing to -h instead.
>
> Print -h help to stdout rather than stderr.  Fix default of -p.  Clean
> up the help text a bit.
>
> Signed-off-by: Markus Armbruster <address@hidden>
> ---

Reviewed-by: Marc-André Lureau <address@hidden>


>  contrib/ivshmem-server/main.c | 63 
> ++++++++++++++++++++++++-------------------
>  1 file changed, 35 insertions(+), 28 deletions(-)
>
> diff --git a/contrib/ivshmem-server/main.c b/contrib/ivshmem-server/main.c
> index cca1061..e9b4388 100644
> --- a/contrib/ivshmem-server/main.c
> +++ b/contrib/ivshmem-server/main.c
> @@ -33,31 +33,32 @@ typedef struct IvshmemServerArgs {
>      unsigned n_vectors;
>  } IvshmemServerArgs;
>
> -/* show ivshmem_server_usage and exit with given error code */
>  static void
> -ivshmem_server_usage(const char *name, int code)
> +ivshmem_server_usage(const char *progname)
>  {
> -    fprintf(stderr, "%s [opts]\n", name);
> -    fprintf(stderr, "  -h: show this help\n");
> -    fprintf(stderr, "  -v: verbose mode\n");
> -    fprintf(stderr, "  -F: foreground mode (default is to daemonize)\n");
> -    fprintf(stderr, "  -p <pid_file>: path to the PID file (used in daemon\n"
> -                    "     mode only).\n"
> -                    "     Default=%s\n", IVSHMEM_SERVER_DEFAULT_SHM_PATH);
> -    fprintf(stderr, "  -S <unix_socket_path>: path to the unix socket\n"
> -                    "     to listen to.\n"
> -                    "     Default=%s\n", 
> IVSHMEM_SERVER_DEFAULT_UNIX_SOCK_PATH);
> -    fprintf(stderr, "  -m <shm_path>: path to the shared memory.\n"
> -                    "     The path corresponds to a POSIX shm name or a\n"
> -                    "     hugetlbfs mount point.\n"
> -                    "     default=%s\n", IVSHMEM_SERVER_DEFAULT_SHM_PATH);
> -    fprintf(stderr, "  -l <size>: size of shared memory in bytes. The 
> suffix\n"
> -                    "     K, M and G can be used (ex: 1K means 1024).\n"
> -                    "     default=%u\n", IVSHMEM_SERVER_DEFAULT_SHM_SIZE);
> -    fprintf(stderr, "  -n <n_vects>: number of vectors.\n"
> -                    "     default=%u\n", IVSHMEM_SERVER_DEFAULT_N_VECTORS);
> +    printf("Usage: %s [OPTION]...\n"
> +           "  -h: show this help\n"
> +           "  -v: verbose mode\n"
> +           "  -F: foreground mode (default is to daemonize)\n"
> +           "  -p <pid_file>: path to the PID file (used in daemon mode 
> only)\n"
> +           "     default " IVSHMEM_SERVER_DEFAULT_PID_FILE "\n"
> +           "  -S <unix_socket_path>: path to the unix socket to listen to\n"
> +           "     default " IVSHMEM_SERVER_DEFAULT_UNIX_SOCK_PATH "\n"
> +           "  -m <shm_path>: POSIX shared memory object name or a hugetlbfs 
> mount point\n"
> +           "     default " IVSHMEM_SERVER_DEFAULT_SHM_PATH "\n"
> +           "  -l <size>: size of shared memory in bytes\n"
> +           "     suffixes K, M and G can be used, e.g. 1K means 1024\n"
> +           "     default %u\n"
> +           "  -n <n_vectors>: number of vectors\n"
> +           "     default %u\n",
> +           progname, IVSHMEM_SERVER_DEFAULT_SHM_SIZE,
> +           IVSHMEM_SERVER_DEFAULT_N_VECTORS);
> +}
>
> -    exit(code);
> +static void
> +ivshmem_server_help(const char *progname)
> +{
> +    fprintf(stderr, "Try '%s -h' for more information.\n", progname);
>  }
>
>  /* parse the program arguments, exit on error */
> @@ -81,7 +82,8 @@ ivshmem_server_parse_args(IvshmemServerArgs *args, int 
> argc, char *argv[])
>
>          switch (c) {
>          case 'h': /* help */
> -            ivshmem_server_usage(argv[0], 0);
> +            ivshmem_server_usage(argv[0]);
> +            exit(0);
>              break;
>
>          case 'v': /* verbose */
> @@ -108,20 +110,23 @@ ivshmem_server_parse_args(IvshmemServerArgs *args, int 
> argc, char *argv[])
>              parse_option_size("shm_size", optarg, &args->shm_size, &err);
>              if (err) {
>                  error_report_err(err);
> -                ivshmem_server_usage(argv[0], 1);
> +                ivshmem_server_help(argv[0]);
> +                exit(1);
>              }
>              break;
>
>          case 'n': /* n_vectors */
>              if (parse_uint_full(optarg, &v, 0) < 0) {
>                  fprintf(stderr, "cannot parse n_vectors\n");
> -                ivshmem_server_usage(argv[0], 1);
> +                ivshmem_server_help(argv[0]);
> +                exit(1);
>              }
>              args->n_vectors = v;
>              break;
>
>          default:
> -            ivshmem_server_usage(argv[0], 1);
> +            ivshmem_server_usage(argv[0]);
> +            exit(1);
>              break;
>          }
>      }
> @@ -129,12 +134,14 @@ ivshmem_server_parse_args(IvshmemServerArgs *args, int 
> argc, char *argv[])
>      if (args->n_vectors > IVSHMEM_SERVER_MAX_VECTORS) {
>          fprintf(stderr, "too many requested vectors (max is %d)\n",
>                  IVSHMEM_SERVER_MAX_VECTORS);
> -        ivshmem_server_usage(argv[0], 1);
> +        ivshmem_server_help(argv[0]);
> +        exit(1);
>      }
>
>      if (args->verbose == 1 && args->foreground == 0) {
>          fprintf(stderr, "cannot use verbose in daemon mode\n");
> -        ivshmem_server_usage(argv[0], 1);
> +        ivshmem_server_help(argv[0]);
> +        exit(1);
>      }
>  }
>
> --
> 2.4.3
>
>



-- 
Marc-André Lureau