On Wed, 12 Jan 2022 08:44:19 -0500
"Michael S. Tsirkin" <m...@redhat.com> wrote:
> On Wed, Jan 12, 2022 at 08:03:29AM -0500, Igor Mammedov wrote:
> > The next commit will revert OEM fields padding with whitespace to
> > padding with '\0' as it was before [1]. As result test_oem_fields() will
> > fail due to unexpectedly smaller ID sizes read from QEMU ACPI tables.
> >
> > Pad OEM_ID/OEM_TABLE_ID manually with spaces so that values the test
> > puts on QEMU CLI and expected values match.
> >
> > 1) 602b458201 ("acpi: Permit OEM ID and OEM table ID fields to be changed")
> > Signed-off-by: Igor Mammedov <imamm...@redhat.com>
>
> That's kind of ugly in that we do not test
> shorter names then. How about we pad with \0 instead?
test_acpi_q35_slic() should cover short OEM_TABLE_ID.
also padding in this patch makes test_oem_fields() cleaner
and simplifies 3/4, switching to \0 here would require
merging this patch with the fix itself to avoid breaking
bisection.
If you still prefer to have test_oem_fields() test short
names, I can post following on top that can to it without
breaking bisection:
diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index 90c9f6a0a2..0fd7cf1f89 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -71,8 +71,8 @@
#define ACPI_REBUILD_EXPECTED_AML "TEST_ACPI_REBUILD_AML"
-#define OEM_ID "TEST "
-#define OEM_TABLE_ID "OEM "
+#define OEM_ID "TEST"
+#define OEM_TABLE_ID "OEM"
#define OEM_TEST_ARGS "-machine x-oem-id='" OEM_ID "',x-oem-table-id='" \
OEM_TABLE_ID "'"
@@ -1530,8 +1530,8 @@ static void test_oem_fields(test_data *data)
continue;
}
- g_assert(memcmp(sdt->aml + 10, OEM_ID, 6) == 0);
- g_assert(memcmp(sdt->aml + 16, OEM_TABLE_ID, 8) == 0);
+ g_assert(strncmp((char *)sdt->aml + 10, OEM_ID, 6) == 0);
+ g_assert(strncmp((char *)sdt->aml + 16, OEM_TABLE_ID, 8) == 0);
}
}
> And add a comment explaining why it's done.
>
> > ---
> > tests/qtest/bios-tables-test.c | 15 ++++++---------
> > 1 file changed, 6 insertions(+), 9 deletions(-)
> >
> > diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> > index e6b72d9026..90c9f6a0a2 100644
> > --- a/tests/qtest/bios-tables-test.c
> > +++ b/tests/qtest/bios-tables-test.c
> > @@ -71,9 +71,10 @@
> >
> > #define ACPI_REBUILD_EXPECTED_AML "TEST_ACPI_REBUILD_AML"
> >
> > -#define OEM_ID "TEST"
> > -#define OEM_TABLE_ID "OEM"
> > -#define OEM_TEST_ARGS "-machine
> > x-oem-id="OEM_ID",x-oem-table-id="OEM_TABLE_ID
> > +#define OEM_ID "TEST "
> > +#define OEM_TABLE_ID "OEM "
> > +#define OEM_TEST_ARGS "-machine x-oem-id='" OEM_ID
> > "',x-oem-table-id='" \
> > + OEM_TABLE_ID "'"
> >
> > typedef struct {
> > bool tcg_only;
> > @@ -1519,11 +1520,7 @@ static void test_acpi_q35_slic(void)
> > static void test_oem_fields(test_data *data)
> > {
> > int i;
> > - char oem_id[6];
> > - char oem_table_id[8];
> >
> > - strpadcpy(oem_id, sizeof oem_id, OEM_ID, ' ');
> > - strpadcpy(oem_table_id, sizeof oem_table_id, OEM_TABLE_ID, ' ');
> > for (i = 0; i < data->tables->len; ++i) {
> > AcpiSdtTable *sdt;
> >
> > @@ -1533,8 +1530,8 @@ static void test_oem_fields(test_data *data)
> > continue;
> > }
> >
> > - g_assert(memcmp(sdt->aml + 10, oem_id, 6) == 0);
> > - g_assert(memcmp(sdt->aml + 16, oem_table_id, 8) == 0);
> > + g_assert(memcmp(sdt->aml + 10, OEM_ID, 6) == 0);
> > + g_assert(memcmp(sdt->aml + 16, OEM_TABLE_ID, 8) == 0);
> > }
> > }
> >
> > --
> > 2.31.1
>
