Invoking the --help option of any tool should not return with an error,
if that tool does indeed 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 '?' makes it allows to handle --help.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 tools/xentrace/xentrace.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

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 = "<mark.a.williamson@intel.com>";
 
-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);
         }
     }

On 01.06.2023 10:08, Olaf Hering wrote:
> Invoking the --help option of any tool should not return with an error,
> if that tool does indeed 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 '?' makes it allows to handle --help.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>

Is this any different from the v1 sent on May 26th?

Jan

> --- 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 = "<mark.a.williamson@intel.com>";
>  
> -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);
>          }
>      }
>  
>

Thu, 1 Jun 2023 10:17:17 +0200 Jan Beulich <jbeulich@suse.com>:

> Is this any different from the v1 sent on May 26th?

Yeah, this one still has the bogus commit message.
I forgot that this patch was already sent earlier.

Please ignore this one.


Olaf