[Spice-devel,server/tests,4/6] server/tests: remove option from usage if AUTOMATED_TESTS is not configured

Submitted by Uri Lublin on July 9, 2013, 4:15 p.m.

Details

Message ID d61c2e9ea3a2069ad021ad14efe50603e54c13f0.1373386364.git.uril@redhat.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Uri Lublin July 9, 2013, 4:15 p.m.
---
 server/tests/test_display_base.c |   25 +++++++++++++++++++------
 1 files changed, 19 insertions(+), 6 deletions(-)

Patch hide | download patch | download mbox

diff --git a/server/tests/test_display_base.c b/server/tests/test_display_base.c
index a4fdae9..3312b11 100644
--- a/server/tests/test_display_base.c
+++ b/server/tests/test_display_base.c
@@ -62,6 +62,7 @@  static int control = 3; //used to know when we can take a screenshot
 static int rects = 16; //number of rects that will be draw
 static int has_automated_tests = 0; //automated test flag
 
+__attribute__((noreturn))
 static void sigchld_handler(int signal_num) // wait for the child process and exit
 {
     int status;
@@ -878,6 +879,19 @@  void init_automated()
     sigaction(SIGCHLD, &sa, NULL);
 }
 
+__attribute__((noreturn))
+void usage(const char *argv0, const int exitcode)
+{
+#ifdef AUTOMATED_TESTS
+    const char *autoopt="[--automated-tests]";
+#else
+    const char *autoopt="";
+#endif
+
+    printf("usage: %s %s\n", argv0, autoopt);
+    exit(exitcode);
+}
+
 void spice_test_config_parse_args(int argc, char **argv)
 {
     struct option options[] = {
@@ -893,19 +907,18 @@  void spice_test_config_parse_args(int argc, char **argv)
         switch (val) {
         case '?':
             printf("unrecognized option '%s'\n", argv[optind - 1]);
-            goto invalid_option;
+            usage(argv[0], EXIT_FAILURE);
         case 0:
             break;
         }
     }
 
+    if (argc > optind) {
+		printf("unknown argument '%s'\n", argv[optind]);
+        usage(argv[0], EXIT_FAILURE);
+	}
     if (has_automated_tests) {
         init_automated();
     }
     return;
-
-invalid_option:
-    printf("Invalid option!\n"
-           "usage: %s [--automated-tests]\n", argv[0]);
-    exit(0);
 }

Comments

On Tue, 2013-07-09 at 19:15 +0300, Uri Lublin wrote:

ACK, just one comment that is nice to fix before pushing.

> ---
>  server/tests/test_display_base.c |   25 +++++++++++++++++++------
>  1 files changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/server/tests/test_display_base.c b/server/tests/test_display_base.c
> index a4fdae9..3312b11 100644
> --- a/server/tests/test_display_base.c
> +++ b/server/tests/test_display_base.c
> @@ -62,6 +62,7 @@ static int control = 3; //used to know when we can take a screenshot
>  static int rects = 16; //number of rects that will be draw
>  static int has_automated_tests = 0; //automated test flag
>  
> +__attribute__((noreturn))
>  static void sigchld_handler(int signal_num) // wait for the child process and exit
>  {
>      int status;
> @@ -878,6 +879,19 @@ void init_automated()
>      sigaction(SIGCHLD, &sa, NULL);
>  }
>  
> +__attribute__((noreturn))
> +void usage(const char *argv0, const int exitcode)
> +{
> +#ifdef AUTOMATED_TESTS
> +    const char *autoopt="[--automated-tests]";

Would be slightly nicer to add the space to the string.

> +#else
> +    const char *autoopt="";
> +#endif
> +
> +    printf("usage: %s %s\n", argv0, autoopt);
> +    exit(exitcode);
> +}
> +
>  void spice_test_config_parse_args(int argc, char **argv)
>  {
>      struct option options[] = {
> @@ -893,19 +907,18 @@ void spice_test_config_parse_args(int argc, char **argv)
>          switch (val) {
>          case '?':
>              printf("unrecognized option '%s'\n", argv[optind - 1]);
> -            goto invalid_option;
> +            usage(argv[0], EXIT_FAILURE);
>          case 0:
>              break;
>          }
>      }
>  
> +    if (argc > optind) {
> +		printf("unknown argument '%s'\n", argv[optind]);
> +        usage(argv[0], EXIT_FAILURE);
> +	}
>      if (has_automated_tests) {
>          init_automated();
>      }
>      return;
> -
> -invalid_option:
> -    printf("Invalid option!\n"
> -           "usage: %s [--automated-tests]\n", argv[0]);
> -    exit(0);
>  }
On 07/14/2013 12:19 PM, Alon Levy wrote:
> On Tue, 2013-07-09 at 19:15 +0300, Uri Lublin wrote:
>
> ACK, just one comment that is nice to fix before pushing.
>
>> diff --git a/server/tests/test_display_base.c b/server/tests/test_display_base.c
>> index a4fdae9..3312b11 100644
>> --- a/server/tests/test_display_base.c
>> +++ b/server/tests/test_display_base.c
>>
>> @@ -878,6 +879,19 @@ void init_automated()
>>       sigaction(SIGCHLD, &sa, NULL);
>>   }
>>   
>> +__attribute__((noreturn))
>> +void usage(const char *argv0, const int exitcode)
>> +{
>> +#ifdef AUTOMATED_TESTS
>> +    const char *autoopt="[--automated-tests]";
> Would be slightly nicer to add the space to the string.

There exists a space before the "%s" in the printf's format below.
Do you mean it would be nicer to have that space in autoopt and
have the printf format "%s%s\n" ?

>
>> +#else
>> +    const char *autoopt="";
>> +#endif
>> +
>> +    printf("usage: %s %s\n", argv0, autoopt);
>> +    exit(exitcode);
>> +}
>> +
On 07/14/2013 03:47 PM, Uri Lublin wrote:
> On 07/14/2013 12:19 PM, Alon Levy wrote:
>> On Tue, 2013-07-09 at 19:15 +0300, Uri Lublin wrote:
>>
>> ACK, just one comment that is nice to fix before pushing.
>>
>>> diff --git a/server/tests/test_display_base.c 
>>> b/server/tests/test_display_base.c
>>> index a4fdae9..3312b11 100644
>>> --- a/server/tests/test_display_base.c
>>> +++ b/server/tests/test_display_base.c
>>>
>>> @@ -878,6 +879,19 @@ void init_automated()
>>>       sigaction(SIGCHLD, &sa, NULL);
>>>   }
>>>   +__attribute__((noreturn))
>>> +void usage(const char *argv0, const int exitcode)
>>> +{
>>> +#ifdef AUTOMATED_TESTS
>>> +    const char *autoopt="[--automated-tests]";
>> Would be slightly nicer to add the space to the string.
>
> There exists a space before the "%s" in the printf's format below.
> Do you mean it would be nicer to have that space in autoopt and
> have the printf format "%s%s\n" ?

Pushed as you've requested with the space in autoopt and the format "%s%s"


>
>>
>>> +#else
>>> +    const char *autoopt="";
>>> +#endif
>>> +
>>> +    printf("usage: %s %s\n", argv0, autoopt);
>>> +    exit(exitcode);
>>> +}
>>> +
>