diff mbox series

[v2,2/4] tests/migration-test: Add a test for null parameter setups

Message ID 20230825171517.1215317-3-peterx@redhat.com
State New
Headers show
Series qapi/migration: Dedup migration parameter objects and fix tls-authz crash | expand

Commit Message

Peter Xu Aug. 25, 2023, 5:15 p.m. UTC 
Add a test for StrOrNull parameters (tls-*).

Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 tests/qtest/migration-test.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

Comments

Thomas Huth Aug. 25, 2023, 5:33 p.m. UTC | #1 
On 25/08/2023 19.15, Peter Xu wrote:
> Add a test for StrOrNull parameters (tls-*).
> 
> Reviewed-by: Fabiano Rosas <farosas@suse.de>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>   tests/qtest/migration-test.c | 21 +++++++++++++++++++++
>   1 file changed, 21 insertions(+)
> 
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index 62d3f37021..64efee8b04 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -1471,6 +1471,26 @@ static void test_postcopy_preempt_all(void)
>   
>   #endif
>   
> +/*
> + * We have a few parameters that allows null as input, test them to make
> + * sure they won't crash (where some used to).
> + */
> +static void test_null_parameters(void)
> +{
> +    const char *allow_null_params[] = {
> +        "tls-authz", "tls-hostname", "tls-creds"};

I'd place the ending bracket on a new line.

> +    QTestState *vm = qtest_init("");
> +    int i;
> +
> +    for (i = 0; i < sizeof(allow_null_params) / sizeof(const char *); i++) {

Could you use ARRAY_SIZE() instead of calculating it on your own?

> +        qtest_qmp_assert_success(vm, "{ 'execute': 'migrate-set-parameters',"
> +                                 "'arguments': { %s: null } }",
> +                                 allow_null_params[i]);
> +    }
> +
> +    qtest_quit(vm);
> +}
> +
>   static void test_baddest(void)
>   {
>       MigrateStart args = {
> @@ -2827,6 +2847,7 @@ int main(int argc, char **argv)
>           }
>       }
>   
> +    qtest_add_func("/migration/null_parameters", test_null_parameters);
>       qtest_add_func("/migration/bad_dest", test_baddest);
>       qtest_add_func("/migration/precopy/unix/plain", test_precopy_unix_plain);
>       qtest_add_func("/migration/precopy/unix/xbzrle", test_precopy_unix_xbzrle);

  Thomas
Peter Xu Aug. 25, 2023, 8:57 p.m. UTC | #2 
On Fri, Aug 25, 2023 at 07:33:23PM +0200, Thomas Huth wrote:
> On 25/08/2023 19.15, Peter Xu wrote:
> > Add a test for StrOrNull parameters (tls-*).
> > 
> > Reviewed-by: Fabiano Rosas <farosas@suse.de>
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >   tests/qtest/migration-test.c | 21 +++++++++++++++++++++
> >   1 file changed, 21 insertions(+)
> > 
> > diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> > index 62d3f37021..64efee8b04 100644
> > --- a/tests/qtest/migration-test.c
> > +++ b/tests/qtest/migration-test.c
> > @@ -1471,6 +1471,26 @@ static void test_postcopy_preempt_all(void)
> >   #endif
> > +/*
> > + * We have a few parameters that allows null as input, test them to make
> > + * sure they won't crash (where some used to).
> > + */
> > +static void test_null_parameters(void)
> > +{
> > +    const char *allow_null_params[] = {
> > +        "tls-authz", "tls-hostname", "tls-creds"};
> 
> I'd place the ending bracket on a new line.
> 
> > +    QTestState *vm = qtest_init("");
> > +    int i;
> > +
> > +    for (i = 0; i < sizeof(allow_null_params) / sizeof(const char *); i++) {
> 
> Could you use ARRAY_SIZE() instead of calculating it on your own?

Sure (on both), thanks.
Philippe Mathieu-Daudé Aug. 29, 2023, 2:12 p.m. UTC | #3 
On 25/8/23 19:15, Peter Xu wrote:
> Add a test for StrOrNull parameters (tls-*).
> 
> Reviewed-by: Fabiano Rosas <farosas@suse.de>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>   tests/qtest/migration-test.c | 21 +++++++++++++++++++++
>   1 file changed, 21 insertions(+)
> 
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index 62d3f37021..64efee8b04 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -1471,6 +1471,26 @@ static void test_postcopy_preempt_all(void)
>   
>   #endif
>   
> +/*
> + * We have a few parameters that allows null as input, test them to make
> + * sure they won't crash (where some used to).
> + */
> +static void test_null_parameters(void)
> +{
> +    const char *allow_null_params[] = {

static

> +        "tls-authz", "tls-hostname", "tls-creds"};

With Thomas comment addressed:

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
diff mbox series

Patch

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 62d3f37021..64efee8b04 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -1471,6 +1471,26 @@  static void test_postcopy_preempt_all(void)
 
 #endif
 
+/*
+ * We have a few parameters that allows null as input, test them to make
+ * sure they won't crash (where some used to).
+ */
+static void test_null_parameters(void)
+{
+    const char *allow_null_params[] = {
+        "tls-authz", "tls-hostname", "tls-creds"};
+    QTestState *vm = qtest_init("");
+    int i;
+
+    for (i = 0; i < sizeof(allow_null_params) / sizeof(const char *); i++) {
+        qtest_qmp_assert_success(vm, "{ 'execute': 'migrate-set-parameters',"
+                                 "'arguments': { %s: null } }",
+                                 allow_null_params[i]);
+    }
+
+    qtest_quit(vm);
+}
+
 static void test_baddest(void)
 {
     MigrateStart args = {
@@ -2827,6 +2847,7 @@  int main(int argc, char **argv)
         }
     }
 
+    qtest_add_func("/migration/null_parameters", test_null_parameters);
     qtest_add_func("/migration/bad_dest", test_baddest);
     qtest_add_func("/migration/precopy/unix/plain", test_precopy_unix_plain);
     qtest_add_func("/migration/precopy/unix/xbzrle", test_precopy_unix_xbzrle);