[v1] xentrace: adjust exit code for --help option

Commit Message

Olaf Hering May 26, 2023, 12:38 p.m. UTC

Invoking the --help option of any tool should not return with an error,
if that tool does have a documented and implemented help option.

Adjust the usage() function to exit with either error or success.
Handle the existing entry in the option table to call usage accordingly.

Adjust the getopt value for help. The char '?' is returned for unknown
options. Returning 'h' instead of '?' allows to handle --help.

Signed-off-by: Olaf Hering <[e-mail]>
---
 tools/xentrace/xentrace.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

Comments

George Dunlap July 28, 2023, 8:53 p.m. UTC | #1

On Fri, May 26, 2023 at 1:38 PM Olaf Hering <[e-mail]> wrote:

> Invoking the --help option of any tool should not return with an error,
> if that tool does have a documented and implemented help option.
>
> Adjust the usage() function to exit with either error or success.
> Handle the existing entry in the option table to call usage accordingly.
>
> Adjust the getopt value for help. The char '?' is returned for unknown
> options. Returning 'h' instead of '?' allows to handle --help.
>
> Signed-off-by: Olaf Hering <[e-mail]>
>

Reviewed-by: George Dunlap <[e-mail]>

Patch

diff --git a/tools/xentrace/xentrace.c b/tools/xentrace/xentrace.c
index 3548255123..be6226f088 100644
--- a/tools/xentrace/xentrace.c
+++ b/tools/xentrace/xentrace.c
@@ -807,7 +807,7 @@  static void monitor_tbufs(void)
 const char *program_version     = "xentrace v1.2";
 const char *program_bug_address = "<[e-mail]>";
 
-static void usage(void)
+static void usage(int status)
 {
 #define USAGE_STR \
 "Usage: xentrace [OPTION...] [output file]\n" \
@@ -854,7 +854,7 @@  static void usage(void)
     printf(USAGE_STR);
     printf("\nReport bugs to %s\n", program_bug_address);
 
-    exit(EXIT_FAILURE);
+    exit(status);
 }
 
 /* convert the argument string pointed to by arg to a long int representation,
@@ -873,7 +873,7 @@  long sargtol(const char *restrict arg, int base)
     {
         fprintf(stderr, "Invalid option argument: %s\n", arg);
         fprintf(stderr, "Error: %s\n\n", strerror(errno));
-        usage();
+        usage(EXIT_FAILURE);
     }
     else if (endp == arg)
     {
@@ -901,7 +901,7 @@  long sargtol(const char *restrict arg, int base)
 
 invalid:
     fprintf(stderr, "Invalid option argument: %s\n\n", arg);
-    usage();
+    usage(EXIT_FAILURE);
     return 0; /* not actually reached */
 }
 
@@ -917,10 +917,10 @@  static long argtol(const char *restrict arg, int base)
     if (errno != 0) {
         fprintf(stderr, "Invalid option argument: %s\n", arg);
         fprintf(stderr, "Error: %s\n\n", strerror(errno));
-        usage();
+        usage(EXIT_FAILURE);
     } else if (endp == arg || *endp != '\0') {
         fprintf(stderr, "Invalid option argument: %s\n\n", arg);
-        usage();
+        usage(EXIT_FAILURE);
     }
 
     return val;
@@ -1090,7 +1090,7 @@  static void parse_args(int argc, char **argv)
         { "discard-buffers", no_argument,      0, 'D' },
         { "dont-disable-tracing", no_argument, 0, 'x' },
         { "start-disabled", no_argument,       0, 'X' },
-        { "help",           no_argument,       0, '?' },
+        { "help",           no_argument,       0, 'h' },
         { "version",        no_argument,       0, 'V' },
         { 0, 0, 0, 0 }
     };
@@ -1144,8 +1144,12 @@  static void parse_args(int argc, char **argv)
             opts.memory_buffer = sargtol(optarg, 0);
             break;
 
+        case 'h':
+            usage(EXIT_SUCCESS);
+            break;
+
         default:
-            usage();
+            usage(EXIT_FAILURE);
         }
     }