Re: [PATCH BlueZ v1 2/3] unit/test-sdp-xml: Give each test its own test_data

Hi Bastien,

On Mon, Aug 17, 2026 at 4:41 AM Bastien Nocera <[email protected]> wrote:
>
> On Fri, 2026-08-14 at 13:47 -0400, Luiz Augusto von Dentz wrote:
> > From: Luiz Augusto von Dentz <[email protected]>
> >
> > DEFINE_TEST assigned the filename and expected result to a single
> > test_data and passed its address to tester_add(), so every test
> > shared
> > one struct and ran with whatever the last registered test left in it.
> >
> > All the file based tests therefore parsed the same file, and a test
> > pointing at a file that does not exist still passed.
> >
> > That in particular meant compute-seq-size-type-confusion.xml, the
> > regression test for GHSA-7mmr-gwqx-vc34, was never actually parsed,
> > and
> > passed only because it inherited the expected result of the test
> > registered after it.
>
> That's not quite correct. The fix introduced memory leaks, which you're
> fixing in "sdp-xml: Fix leaking the parse stack on malformed input",
> but it fixed the original crash.
>
> This test would only fail when run under ASan.
>
> I just want to be clear that this fix doesn't have an impact on GHSA-
> 7mmr-gwqx-vc34 being resolved earlier.

Yeah, the model got confused, it seems it doesn't understand the macro
expands the fname, I though it was off or something because it didn't
seem to change anything with respect to the output.

> >
> > Use an array with one entry per test instead.
> >
> > Assisted-by: Claude:claude-opus-5
> > ---
> >  unit/test-sdp-xml.c | 34 ++++++++++++++++++++++------------
> >  1 file changed, 22 insertions(+), 12 deletions(-)
> >
> > diff --git a/unit/test-sdp-xml.c b/unit/test-sdp-xml.c
> > index b338788aa295..72e01f5f5806 100644
> > --- a/unit/test-sdp-xml.c
> > +++ b/unit/test-sdp-xml.c
> > @@ -114,25 +114,35 @@ static void
> > sequence_on_squared_teardown(gconstpointer data)
> >       tester_teardown_complete();
> >  }
> >
> > -#define DEFINE_TEST(fname, res)
> > {                                     \
> > -     data.expected_result =
> > res;                                  \
> > -     data.filename =
> > fname;                                                \
> > -     tester_add("/" fname, &data,
> > NULL,                         \
> > -                     parse_xml_for_filename,
> > NULL);                        \
> > -     }
> > +#define DEFINE_TEST(fname, res) { .filename = fname,
> > .expected_result = res }
> > +
> > +/*
> > + * Each test needs its own test_data, sharing a single one would
> > make every
> > + * test run with the values assigned by the last one registered.
> > + */
> > +static struct test_data file_tests[] = {
> > +     DEFINE_TEST("Bluetooth_HID-sdp_record.xml", TRUE),
> > +     DEFINE_TEST("qt-SerialPortSDPRecord.xml", TRUE),
> > +     /* From
> > https://github.com/bluez/bluez/security/advisories/GHSA-7mmr-gwqx-vc34
> >  */
> > +     DEFINE_TEST("compute-seq-size-type-confusion.xml", FALSE),
> > +     /* From
> > https://github.com/bluez/bluez/security/advisories/GHSA-75v6-6q44-57hc
> >  */
> > +     DEFINE_TEST("duplicate-attribute.xml", TRUE),
> > +};
> >
> >  int main(int argc, char *argv[])
> >  {
> >       struct test_data data;
> > +     unsigned int i;
> >
> >       tester_init(&argc, &argv);
> >
> > -     DEFINE_TEST("Bluetooth_HID-sdp_record.xml", TRUE);
> > -     DEFINE_TEST("qt-SerialPortSDPRecord.xml", TRUE);
> > -     /* From
> > https://github.com/bluez/bluez/security/advisories/GHSA-7mmr-gwqx-vc34
> >  */
> > -     DEFINE_TEST("compute-seq-size-type-confusion.xml", FALSE);
> > -     /* From
> > https://github.com/bluez/bluez/security/advisories/GHSA-75v6-6q44-57hc
> >  */
> > -     DEFINE_TEST("duplicate-attribute.xml", TRUE);
> > +     for (i = 0; i < G_N_ELEMENTS(file_tests); i++) {
> > +             char *name = g_strdup_printf("/%s",
> > file_tests[i].filename);
> > +
> > +             tester_add(name, &file_tests[i], NULL,
> > +                             parse_xml_for_filename, NULL);
> > +             g_free(name);
> > +     }
> >
> >       tester_add("/sequence_on_squared", &data,
> >                  sequence_on_squared_setup,



-- 
Luiz Augusto von Dentz