From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37742) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W0A80-0001fR-45 for qemu-devel@nongnu.org; Mon, 06 Jan 2014 08:22:05 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W0A7v-0002Rc-1x for qemu-devel@nongnu.org; Mon, 06 Jan 2014 08:22:00 -0500 Received: from mx1.redhat.com ([209.132.183.28]:50093) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W0A7u-0002RM-Q0 for qemu-devel@nongnu.org; Mon, 06 Jan 2014 08:21:54 -0500 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s06DLqTT018869 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 6 Jan 2014 08:21:53 -0500 Message-ID: <1389014512.2097.7.camel@localhost.localdomain> From: Marcel Apfelbaum Date: Mon, 06 Jan 2014 15:21:52 +0200 In-Reply-To: <20140106114309.GA29689@redhat.com> References: <1388320370-28732-1-git-send-email-marcel.a@redhat.com> <20140106114309.GA29689@redhat.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] acpi unit-test: resolved iasl crash List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: qemu-devel@nongnu.org On Mon, 2014-01-06 at 13:43 +0200, Michael S. Tsirkin wrote: > On Sun, Dec 29, 2013 at 02:32:50PM +0200, Marcel Apfelbaum wrote: > > It seems that iasl has an issue when disassembles > > some ACPI tables using the command line: > > iasl -e DSDT -e SSDT -d HPET > > I opened a bug on iasl project: > > https://github.com/acpica/acpica/issues/20 > > > > Modified the iasl command line to "iasl -d HPET" > > until the problem is solved. The command line > > remained the same for DSDT and SSDT tables. > > > > Reported-by:Michael S. Tsirkin > > space needed after : Sure, missed that > > > Signed-off-by: Marcel Apfelbaum > > --- > > 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 2d32b69..26d2e26 100644 > > --- a/tests/acpi-test.c > > +++ b/tests/acpi-test.c > > @@ -403,7 +403,7 @@ static void dump_aml_files(test_data *data, bool rebuild) > > } > > } > > > > -static void load_asl(GArray *sdts, AcpiSdtTable *sdt) > > +static void load_asl(GArray *sdts, AcpiSdtTable *sdt, bool referenceDsdt) > > { > > AcpiSdtTable *temp; > > GError *error = NULL; > > @@ -419,9 +419,11 @@ static void load_asl(GArray *sdts, AcpiSdtTable *sdt) > > > > /* build command line */ > > g_string_append_printf(command_line, " -p %s ", sdt->asl_file); > > - for (i = 0; i < 2; ++i) { /* reference DSDT and SSDT */ > > - temp = &g_array_index(sdts, AcpiSdtTable, i); > > - g_string_append_printf(command_line, "-e %s ", temp->aml_file); > > + if (referenceDsdt) { > > + for (i = 0; i < 2; ++i) { /* reference DSDT and SSDT */ > > + temp = &g_array_index(sdts, AcpiSdtTable, i); > > + g_string_append_printf(command_line, "-e %s ", temp->aml_file); > > + } > > } > > g_string_append_printf(command_line, "-d %s", sdt->aml_file); > > > > @@ -510,14 +512,14 @@ static void test_acpi_asl(test_data *data) > > dump_aml_files(data, false); > > for (i = 0; i < data->ssdt_tables->len; ++i) { > > GString *asl, *exp_asl; > > - > > + bool referenceDsdt = (i < 2); /* only for DSDT and SSDT */ > > > Please don't use camelCase for variables. Sure, no idea why I did that in the first place... > Can we check the table signature instead of looking at the index? It is possible, but not really necessary because: 1. The array of expected tables it is created as a shadow of the original tables. (We load the expected tables in the exact order as the originals) 2. The loading is done based on table signatures. So going over the index in *this* case it means going over the signatures. Thanks for the review, Marcel > > > sdt = &g_array_index(data->ssdt_tables, AcpiSdtTable, i); > > exp_sdt = &g_array_index(exp_data.ssdt_tables, AcpiSdtTable, i); > > > > - load_asl(data->ssdt_tables, sdt); > > + load_asl(data->ssdt_tables, sdt, referenceDsdt); > > asl = normalize_asl(sdt->asl); > > > > - load_asl(exp_data.ssdt_tables, exp_sdt); > > + load_asl(exp_data.ssdt_tables, exp_sdt, referenceDsdt); > > exp_asl = normalize_asl(exp_sdt->asl); > > > > g_assert(!g_strcmp0(asl->str, exp_asl->str)); > > -- > > 1.8.3.1