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

[Philippe Mathieu-Daudé]

On 25/8/23 19:15, Peter Xu wrote:

Add a test for StrOrNull parameters (tls-*).

Reviewed-by: Fabiano Rosas 
Signed-off-by: Peter Xu 
---
  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é 

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

[Peter Xu]

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 
> > Signed-off-by: Peter Xu 
> > ---
> >   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.

-- 
Peter Xu

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

[Thomas Huth]

On 25/08/2023 19.15, Peter Xu wrote:

Add a test for StrOrNull parameters (tls-*).

Reviewed-by: Fabiano Rosas 
Signed-off-by: Peter Xu 
---
  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

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

[Peter Xu]

Add a test for StrOrNull parameters (tls-*).

Reviewed-by: Fabiano Rosas 
Signed-off-by: Peter Xu 
---
 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"};
+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);
-- 
2.41.0