[Qemu-devel] [PATCH v2 0/2] acpi-test: issue errors instead of warnings when possible

[Qemu-devel] [PATCH v2 0/2] acpi-test: issue errors instead of warnings when possible

[Marcel Apfelbaum]

v1->v2:
    Addressed Michael S. Tsirkin's comments:
    - Split the path into 2 patches
      - First patch retains both asl and aml files on failure.
      - Second one "guesses" that iasl installation is OK
        by checking that no error was returned when the
        expected tables were parsed.

Marcel Apfelbaum (2):
  acpi-test: retain both asl and aml files on failure
  acpi-test: issue errors instead of warnings when possible

 tests/acpi-test.c | 40 +++++++++++++++++++++++++---------------
 1 file changed, 25 insertions(+), 15 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH v2 1/2] acpi-test: retain both asl and aml files on failure

[Marcel Apfelbaum]

Updated the error message while at it.

Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
 tests/acpi-test.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/tests/acpi-test.c b/tests/acpi-test.c
index 31f5359..2ce8c18 100644
--- a/tests/acpi-test.c
+++ b/tests/acpi-test.c
@@ -34,7 +34,7 @@ typedef struct {
     gchar *asl;            /* asl code generated from aml */
     gsize asl_len;
     gchar *asl_file;
-    bool asl_file_retain;   /* do not delete the temp asl */
+    bool tmp_files_retain;   /* do not delete the temp asl/aml */
 } QEMU_PACKED AcpiSdtTable;
 
 typedef struct {
@@ -153,7 +153,8 @@ static void free_test_data(test_data *data)
             g_free(temp->aml);
         }
         if (temp->aml_file) {
-            if (g_strstr_len(temp->aml_file, -1, "aml-")) {
+            if (!temp->tmp_files_retain &&
+                g_strstr_len(temp->aml_file, -1, "aml-")) {
                 unlink(temp->aml_file);
             }
             g_free(temp->aml_file);
@@ -162,7 +163,7 @@ static void free_test_data(test_data *data)
             g_free(temp->asl);
         }
         if (temp->asl_file) {
-            if (!temp->asl_file_retain) {
+            if (!temp->tmp_files_retain) {
                 unlink(temp->asl_file);
             }
             g_free(temp->asl_file);
@@ -534,13 +535,14 @@ static void test_acpi_asl(test_data *data)
         exp_asl = normalize_asl(exp_sdt->asl);
 
         if (g_strcmp0(asl->str, exp_asl->str)) {
-            sdt->asl_file_retain = true;
-            exp_sdt->asl_file_retain = true;
+            sdt->tmp_files_retain = true;
+            exp_sdt->tmp_files_retain = true;
             fprintf(stderr,
                     "acpi-test: Warning! %.4s mismatch. "
-                    "Orig asl: %s, expected asl %s.\n",
+                    "Actual [asl:%s, aml:%s], Expected [asl:%s, aml:%s].\n",
                     (gchar *)&exp_sdt->header.signature,
-                    sdt->asl_file, exp_sdt->asl_file);
+                    sdt->asl_file, sdt->aml_file,
+                    exp_sdt->asl_file, exp_sdt->aml_file);
         }
         g_string_free(asl, true);
         g_string_free(exp_asl, true);
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 2/2] acpi-test: issue errors instead of warnings when possible

[Marcel Apfelbaum]

If the expected (offline) acpi tables loaded correctly,
it is safe to assume the iasl installation is OK and
issue an error if the actual tables differ from expected
ones.

Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
 tests/acpi-test.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/tests/acpi-test.c b/tests/acpi-test.c
index 2ce8c18..639b3ab 100644
--- a/tests/acpi-test.c
+++ b/tests/acpi-test.c
@@ -411,7 +411,7 @@ static bool compare_signature(AcpiSdtTable *sdt, uint32_t signature)
    return sdt->header.signature == signature;
 }
 
-static void load_asl(GArray *sdts, AcpiSdtTable *sdt)
+static bool load_asl(GArray *sdts, AcpiSdtTable *sdt)
 {
     AcpiSdtTable *temp;
     GError *error = NULL;
@@ -440,18 +440,22 @@ static void load_asl(GArray *sdts, AcpiSdtTable *sdt)
     g_string_append_printf(command_line, "-d %s", sdt->aml_file);
 
     /* pass 'out' and 'out_err' in order to be redirected */
-    g_spawn_command_line_sync(command_line->str, &out, &out_err, NULL, &error);
+    ret = g_spawn_command_line_sync(command_line->str, &out, &out_err, NULL, &error);
     g_assert_no_error(error);
 
-    ret = g_file_get_contents(sdt->asl_file, (gchar **)&sdt->asl,
-                              &sdt->asl_len, &error);
-    g_assert(ret);
-    g_assert_no_error(error);
-    g_assert(sdt->asl_len);
+    if (ret) {
+        ret = g_file_get_contents(sdt->asl_file, (gchar **)&sdt->asl,
+                                  &sdt->asl_len, &error);
+        g_assert(ret);
+        g_assert_no_error(error);
+        g_assert(sdt->asl_len);
+    }
 
     g_free(out);
     g_free(out_err);
     g_string_free(command_line, true);
+
+    return !ret;
 }
 
 #define COMMENT_END "*/"
@@ -518,6 +522,7 @@ static void test_acpi_asl(test_data *data)
     int i;
     AcpiSdtTable *sdt, *exp_sdt;
     test_data exp_data;
+    gboolean err;
 
     memset(&exp_data, 0, sizeof(exp_data));
     exp_data.tables = load_expected_aml(data);
@@ -531,7 +536,7 @@ static void test_acpi_asl(test_data *data)
         load_asl(data->tables, sdt);
         asl = normalize_asl(sdt->asl);
 
-        load_asl(exp_data.tables, exp_sdt);
+        err = load_asl(exp_data.tables, exp_sdt);
         exp_asl = normalize_asl(exp_sdt->asl);
 
         if (g_strcmp0(asl->str, exp_asl->str)) {
@@ -543,6 +548,9 @@ static void test_acpi_asl(test_data *data)
                     (gchar *)&exp_sdt->header.signature,
                     sdt->asl_file, sdt->aml_file,
                     exp_sdt->asl_file, exp_sdt->aml_file);
+            if (!err) { /* expected data loaded, iasl OK */
+                g_assert(false);
+            }
         }
         g_string_free(asl, true);
         g_string_free(exp_asl, true);
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 2/2] acpi-test: issue errors instead of warnings when possible

[Michael S. Tsirkin]

On Mon, Feb 24, 2014 at 02:09:18PM +0200, Marcel Apfelbaum wrote:
> If the expected (offline) acpi tables loaded correctly,
> it is safe to assume the iasl installation is OK and
> issue an error if the actual tables differ from expected
> ones.
> 
> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>

I'm not sure I agree with this one.
It turned out to be too aggressive in the past
as expected files get out of sync sometimes.
What I would do is this:
	if IASL did not produce errors or warnings
	on expected files, it should not produce
	them on actual files either.



> ---
>  tests/acpi-test.c | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/tests/acpi-test.c b/tests/acpi-test.c
> index 2ce8c18..639b3ab 100644
> --- a/tests/acpi-test.c
> +++ b/tests/acpi-test.c
> @@ -411,7 +411,7 @@ static bool compare_signature(AcpiSdtTable *sdt, uint32_t signature)
>     return sdt->header.signature == signature;
>  }
>  
> -static void load_asl(GArray *sdts, AcpiSdtTable *sdt)
> +static bool load_asl(GArray *sdts, AcpiSdtTable *sdt)
>  {
>      AcpiSdtTable *temp;
>      GError *error = NULL;
> @@ -440,18 +440,22 @@ static void load_asl(GArray *sdts, AcpiSdtTable *sdt)
>      g_string_append_printf(command_line, "-d %s", sdt->aml_file);
>  
>      /* pass 'out' and 'out_err' in order to be redirected */
> -    g_spawn_command_line_sync(command_line->str, &out, &out_err, NULL, &error);
> +    ret = g_spawn_command_line_sync(command_line->str, &out, &out_err, NULL, &error);
>      g_assert_no_error(error);
>  
> -    ret = g_file_get_contents(sdt->asl_file, (gchar **)&sdt->asl,
> -                              &sdt->asl_len, &error);
> -    g_assert(ret);
> -    g_assert_no_error(error);
> -    g_assert(sdt->asl_len);
> +    if (ret) {
> +        ret = g_file_get_contents(sdt->asl_file, (gchar **)&sdt->asl,
> +                                  &sdt->asl_len, &error);
> +        g_assert(ret);
> +        g_assert_no_error(error);
> +        g_assert(sdt->asl_len);
> +    }
>  
>      g_free(out);
>      g_free(out_err);
>      g_string_free(command_line, true);
> +
> +    return !ret;
>  }
>  
>  #define COMMENT_END "*/"
> @@ -518,6 +522,7 @@ static void test_acpi_asl(test_data *data)
>      int i;
>      AcpiSdtTable *sdt, *exp_sdt;
>      test_data exp_data;
> +    gboolean err;
>  
>      memset(&exp_data, 0, sizeof(exp_data));
>      exp_data.tables = load_expected_aml(data);
> @@ -531,7 +536,7 @@ static void test_acpi_asl(test_data *data)
>          load_asl(data->tables, sdt);
>          asl = normalize_asl(sdt->asl);
>  
> -        load_asl(exp_data.tables, exp_sdt);
> +        err = load_asl(exp_data.tables, exp_sdt);
>          exp_asl = normalize_asl(exp_sdt->asl);
>  
>          if (g_strcmp0(asl->str, exp_asl->str)) {
> @@ -543,6 +548,9 @@ static void test_acpi_asl(test_data *data)
>                      (gchar *)&exp_sdt->header.signature,
>                      sdt->asl_file, sdt->aml_file,
>                      exp_sdt->asl_file, exp_sdt->aml_file);
> +            if (!err) { /* expected data loaded, iasl OK */
> +                g_assert(false);
> +            }
>          }
>          g_string_free(asl, true);
>          g_string_free(exp_asl, true);
> -- 
> 1.8.3.1

Re: [Qemu-devel] [PATCH v2 2/2] acpi-test: issue errors instead of warnings when possible

[Marcel Apfelbaum]

On Wed, 2014-02-26 at 17:25 +0200, Michael S. Tsirkin wrote:
> On Mon, Feb 24, 2014 at 02:09:18PM +0200, Marcel Apfelbaum wrote:
> > If the expected (offline) acpi tables loaded correctly,
> > it is safe to assume the iasl installation is OK and
> > issue an error if the actual tables differ from expected
> > ones.
> > 
> > Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> 
> I'm not sure I agree with this one.
> It turned out to be too aggressive in the past
> as expected files get out of sync sometimes.
> What I would do is this:
> 	if IASL did not produce errors or warnings
> 	on expected files, it should not produce
> 	them on actual files either.
The whole point of this patch (series) was to find
a way to discover "real" errors in some cases (not in all).
What you suggested above removes the only "discoverable error"
and returns us to warning only mode.

However, the code does not look into stdout/stderr for iasl
errors (it would involve parsing because iasl *always* writes
to both), but checks if the invocation of iasl command terminates successfully,
merely saying: "Well, iasl invocation succeeded  for the expected files, there
is no reason to fail for the actual ones". The actual output is not verified.

No impact on "out of sync expected files".

Thanks,
Marcel

 

> 
> 
> > ---
> >  tests/acpi-test.c | 24 ++++++++++++++++--------
> >  1 file changed, 16 insertions(+), 8 deletions(-)
> > 
> > diff --git a/tests/acpi-test.c b/tests/acpi-test.c
> > index 2ce8c18..639b3ab 100644
> > --- a/tests/acpi-test.c
> > +++ b/tests/acpi-test.c
> > @@ -411,7 +411,7 @@ static bool compare_signature(AcpiSdtTable *sdt, uint32_t signature)
> >     return sdt->header.signature == signature;
> >  }
> >  
> > -static void load_asl(GArray *sdts, AcpiSdtTable *sdt)
> > +static bool load_asl(GArray *sdts, AcpiSdtTable *sdt)
> >  {
> >      AcpiSdtTable *temp;
> >      GError *error = NULL;
> > @@ -440,18 +440,22 @@ static void load_asl(GArray *sdts, AcpiSdtTable *sdt)
> >      g_string_append_printf(command_line, "-d %s", sdt->aml_file);
> >  
> >      /* pass 'out' and 'out_err' in order to be redirected */
> > -    g_spawn_command_line_sync(command_line->str, &out, &out_err, NULL, &error);
> > +    ret = g_spawn_command_line_sync(command_line->str, &out, &out_err, NULL, &error);
> >      g_assert_no_error(error);
> >  
> > -    ret = g_file_get_contents(sdt->asl_file, (gchar **)&sdt->asl,
> > -                              &sdt->asl_len, &error);
> > -    g_assert(ret);
> > -    g_assert_no_error(error);
> > -    g_assert(sdt->asl_len);
> > +    if (ret) {
> > +        ret = g_file_get_contents(sdt->asl_file, (gchar **)&sdt->asl,
> > +                                  &sdt->asl_len, &error);
> > +        g_assert(ret);
> > +        g_assert_no_error(error);
> > +        g_assert(sdt->asl_len);
> > +    }
> >  
> >      g_free(out);
> >      g_free(out_err);
> >      g_string_free(command_line, true);
> > +
> > +    return !ret;
> >  }
> >  
> >  #define COMMENT_END "*/"
> > @@ -518,6 +522,7 @@ static void test_acpi_asl(test_data *data)
> >      int i;
> >      AcpiSdtTable *sdt, *exp_sdt;
> >      test_data exp_data;
> > +    gboolean err;
> >  
> >      memset(&exp_data, 0, sizeof(exp_data));
> >      exp_data.tables = load_expected_aml(data);
> > @@ -531,7 +536,7 @@ static void test_acpi_asl(test_data *data)
> >          load_asl(data->tables, sdt);
> >          asl = normalize_asl(sdt->asl);
> >  
> > -        load_asl(exp_data.tables, exp_sdt);
> > +        err = load_asl(exp_data.tables, exp_sdt);
> >          exp_asl = normalize_asl(exp_sdt->asl);
> >  
> >          if (g_strcmp0(asl->str, exp_asl->str)) {
> > @@ -543,6 +548,9 @@ static void test_acpi_asl(test_data *data)
> >                      (gchar *)&exp_sdt->header.signature,
> >                      sdt->asl_file, sdt->aml_file,
> >                      exp_sdt->asl_file, exp_sdt->aml_file);
> > +            if (!err) { /* expected data loaded, iasl OK */
> > +                g_assert(false);
> > +            }
> >          }
> >          g_string_free(asl, true);
> >          g_string_free(exp_asl, true);
> > -- 
> > 1.8.3.1

Re: [Qemu-devel] [PATCH v2 2/2] acpi-test: issue errors instead of warnings when possible

[Michael S. Tsirkin]

On Wed, Feb 26, 2014 at 05:39:39PM +0200, Marcel Apfelbaum wrote:
> On Wed, 2014-02-26 at 17:25 +0200, Michael S. Tsirkin wrote:
> > On Mon, Feb 24, 2014 at 02:09:18PM +0200, Marcel Apfelbaum wrote:
> > > If the expected (offline) acpi tables loaded correctly,
> > > it is safe to assume the iasl installation is OK and
> > > issue an error if the actual tables differ from expected
> > > ones.
> > > 
> > > Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> > 
> > I'm not sure I agree with this one.
> > It turned out to be too aggressive in the past
> > as expected files get out of sync sometimes.
> > What I would do is this:
> > 	if IASL did not produce errors or warnings
> > 	on expected files, it should not produce
> > 	them on actual files either.
> The whole point of this patch (series) was to find
> a way to discover "real" errors in some cases (not in all).
> What you suggested above removes the only "discoverable error"
> and returns us to warning only mode.
> 
> However, the code does not look into stdout/stderr for iasl
> errors (it would involve parsing because iasl *always* writes
> to both), but checks if the invocation of iasl command terminates successfully,
> merely saying: "Well, iasl invocation succeeded  for the expected files, there
> is no reason to fail for the actual ones". The actual output is not verified.
> 
> No impact on "out of sync expected files".
> 
> Thanks,
> Marcel
> 

aha
what confused me is this:
            if (!err) { /* expected data loaded, iasl OK */
               g_assert(false);
            }

how about

	g_assert(err);

instead?

> 
> > 
> > 
> > > ---
> > >  tests/acpi-test.c | 24 ++++++++++++++++--------
> > >  1 file changed, 16 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/tests/acpi-test.c b/tests/acpi-test.c
> > > index 2ce8c18..639b3ab 100644
> > > --- a/tests/acpi-test.c
> > > +++ b/tests/acpi-test.c
> > > @@ -411,7 +411,7 @@ static bool compare_signature(AcpiSdtTable *sdt, uint32_t signature)
> > >     return sdt->header.signature == signature;
> > >  }
> > >  
> > > -static void load_asl(GArray *sdts, AcpiSdtTable *sdt)
> > > +static bool load_asl(GArray *sdts, AcpiSdtTable *sdt)
> > >  {
> > >      AcpiSdtTable *temp;
> > >      GError *error = NULL;
> > > @@ -440,18 +440,22 @@ static void load_asl(GArray *sdts, AcpiSdtTable *sdt)
> > >      g_string_append_printf(command_line, "-d %s", sdt->aml_file);
> > >  
> > >      /* pass 'out' and 'out_err' in order to be redirected */
> > > -    g_spawn_command_line_sync(command_line->str, &out, &out_err, NULL, &error);
> > > +    ret = g_spawn_command_line_sync(command_line->str, &out, &out_err, NULL, &error);
> > >      g_assert_no_error(error);
> > >  
> > > -    ret = g_file_get_contents(sdt->asl_file, (gchar **)&sdt->asl,
> > > -                              &sdt->asl_len, &error);
> > > -    g_assert(ret);
> > > -    g_assert_no_error(error);
> > > -    g_assert(sdt->asl_len);
> > > +    if (ret) {
> > > +        ret = g_file_get_contents(sdt->asl_file, (gchar **)&sdt->asl,
> > > +                                  &sdt->asl_len, &error);
> > > +        g_assert(ret);
> > > +        g_assert_no_error(error);
> > > +        g_assert(sdt->asl_len);
> > > +    }
> > >  
> > >      g_free(out);
> > >      g_free(out_err);
> > >      g_string_free(command_line, true);
> > > +
> > > +    return !ret;
> > >  }
> > >  
> > >  #define COMMENT_END "*/"
> > > @@ -518,6 +522,7 @@ static void test_acpi_asl(test_data *data)
> > >      int i;
> > >      AcpiSdtTable *sdt, *exp_sdt;
> > >      test_data exp_data;
> > > +    gboolean err;
> > >  
> > >      memset(&exp_data, 0, sizeof(exp_data));
> > >      exp_data.tables = load_expected_aml(data);
> > > @@ -531,7 +536,7 @@ static void test_acpi_asl(test_data *data)
> > >          load_asl(data->tables, sdt);
> > >          asl = normalize_asl(sdt->asl);
> > >  
> > > -        load_asl(exp_data.tables, exp_sdt);
> > > +        err = load_asl(exp_data.tables, exp_sdt);
> > >          exp_asl = normalize_asl(exp_sdt->asl);
> > >  
> > >          if (g_strcmp0(asl->str, exp_asl->str)) {
> > > @@ -543,6 +548,9 @@ static void test_acpi_asl(test_data *data)
> > >                      (gchar *)&exp_sdt->header.signature,
> > >                      sdt->asl_file, sdt->aml_file,
> > >                      exp_sdt->asl_file, exp_sdt->aml_file);
> > > +            if (!err) { /* expected data loaded, iasl OK */
> > > +                g_assert(false);
> > > +            }
> > >          }
> > >          g_string_free(asl, true);
> > >          g_string_free(exp_asl, true);
> > > -- 
> > > 1.8.3.1
> 
> 

Re: [Qemu-devel] [PATCH v2 2/2] acpi-test: issue errors instead of warnings when possible

[Marcel Apfelbaum]

On Wed, 2014-02-26 at 17:52 +0200, Michael S. Tsirkin wrote:
> On Wed, Feb 26, 2014 at 05:39:39PM +0200, Marcel Apfelbaum wrote:
> > On Wed, 2014-02-26 at 17:25 +0200, Michael S. Tsirkin wrote:
> > > On Mon, Feb 24, 2014 at 02:09:18PM +0200, Marcel Apfelbaum wrote:
> > > > If the expected (offline) acpi tables loaded correctly,
> > > > it is safe to assume the iasl installation is OK and
> > > > issue an error if the actual tables differ from expected
> > > > ones.
> > > > 
> > > > Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> > > 
> > > I'm not sure I agree with this one.
> > > It turned out to be too aggressive in the past
> > > as expected files get out of sync sometimes.
> > > What I would do is this:
> > > 	if IASL did not produce errors or warnings
> > > 	on expected files, it should not produce
> > > 	them on actual files either.
> > The whole point of this patch (series) was to find
> > a way to discover "real" errors in some cases (not in all).
> > What you suggested above removes the only "discoverable error"
> > and returns us to warning only mode.
> > 
> > However, the code does not look into stdout/stderr for iasl
> > errors (it would involve parsing because iasl *always* writes
> > to both), but checks if the invocation of iasl command terminates successfully,
> > merely saying: "Well, iasl invocation succeeded  for the expected files, there
> > is no reason to fail for the actual ones". The actual output is not verified.
> > 
> > No impact on "out of sync expected files".
> > 
> > Thanks,
> > Marcel
> > 
> 
> aha
> what confused me is this:
>             if (!err) { /* expected data loaded, iasl OK */
>                g_assert(false);
>             }
> 
> how about
> 
> 	g_assert(err);
> 
> instead?
You are right!
I'll send a new version with this fix.

Thanks,
Marcel
> 
> > 
> > > 
> > > 
> > > > ---
> > > >  tests/acpi-test.c | 24 ++++++++++++++++--------
> > > >  1 file changed, 16 insertions(+), 8 deletions(-)
> > > > 
> > > > diff --git a/tests/acpi-test.c b/tests/acpi-test.c
> > > > index 2ce8c18..639b3ab 100644
> > > > --- a/tests/acpi-test.c
> > > > +++ b/tests/acpi-test.c
> > > > @@ -411,7 +411,7 @@ static bool compare_signature(AcpiSdtTable *sdt, uint32_t signature)
> > > >     return sdt->header.signature == signature;
> > > >  }
> > > >  
> > > > -static void load_asl(GArray *sdts, AcpiSdtTable *sdt)
> > > > +static bool load_asl(GArray *sdts, AcpiSdtTable *sdt)
> > > >  {
> > > >      AcpiSdtTable *temp;
> > > >      GError *error = NULL;
> > > > @@ -440,18 +440,22 @@ static void load_asl(GArray *sdts, AcpiSdtTable *sdt)
> > > >      g_string_append_printf(command_line, "-d %s", sdt->aml_file);
> > > >  
> > > >      /* pass 'out' and 'out_err' in order to be redirected */
> > > > -    g_spawn_command_line_sync(command_line->str, &out, &out_err, NULL, &error);
> > > > +    ret = g_spawn_command_line_sync(command_line->str, &out, &out_err, NULL, &error);
> > > >      g_assert_no_error(error);
> > > >  
> > > > -    ret = g_file_get_contents(sdt->asl_file, (gchar **)&sdt->asl,
> > > > -                              &sdt->asl_len, &error);
> > > > -    g_assert(ret);
> > > > -    g_assert_no_error(error);
> > > > -    g_assert(sdt->asl_len);
> > > > +    if (ret) {
> > > > +        ret = g_file_get_contents(sdt->asl_file, (gchar **)&sdt->asl,
> > > > +                                  &sdt->asl_len, &error);
> > > > +        g_assert(ret);
> > > > +        g_assert_no_error(error);
> > > > +        g_assert(sdt->asl_len);
> > > > +    }
> > > >  
> > > >      g_free(out);
> > > >      g_free(out_err);
> > > >      g_string_free(command_line, true);
> > > > +
> > > > +    return !ret;
> > > >  }
> > > >  
> > > >  #define COMMENT_END "*/"
> > > > @@ -518,6 +522,7 @@ static void test_acpi_asl(test_data *data)
> > > >      int i;
> > > >      AcpiSdtTable *sdt, *exp_sdt;
> > > >      test_data exp_data;
> > > > +    gboolean err;
> > > >  
> > > >      memset(&exp_data, 0, sizeof(exp_data));
> > > >      exp_data.tables = load_expected_aml(data);
> > > > @@ -531,7 +536,7 @@ static void test_acpi_asl(test_data *data)
> > > >          load_asl(data->tables, sdt);
> > > >          asl = normalize_asl(sdt->asl);
> > > >  
> > > > -        load_asl(exp_data.tables, exp_sdt);
> > > > +        err = load_asl(exp_data.tables, exp_sdt);
> > > >          exp_asl = normalize_asl(exp_sdt->asl);
> > > >  
> > > >          if (g_strcmp0(asl->str, exp_asl->str)) {
> > > > @@ -543,6 +548,9 @@ static void test_acpi_asl(test_data *data)
> > > >                      (gchar *)&exp_sdt->header.signature,
> > > >                      sdt->asl_file, sdt->aml_file,
> > > >                      exp_sdt->asl_file, exp_sdt->aml_file);
> > > > +            if (!err) { /* expected data loaded, iasl OK */
> > > > +                g_assert(false);
> > > > +            }
> > > >          }
> > > >          g_string_free(asl, true);
> > > >          g_string_free(exp_asl, true);
> > > > -- 
> > > > 1.8.3.1
> > 
> >