| Message ID | 1457378754-21649-5-git-send-email-armbru@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Mar 7, 2016 at 8:25 PM, Markus Armbruster <armbru@redhat.com> 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 <armbru@redhat.com> > --- Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > 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 > >
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); } }
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 <armbru@redhat.com> --- contrib/ivshmem-server/main.c | 63 ++++++++++++++++++++++++------------------- 1 file changed, 35 insertions(+), 28 deletions(-)