[PATCH v2 wayland 3/5] scanner: support "since" attribute for enum entries
Peter Hutterer
peter.hutterer at who-t.net
Mon Jan 23 23:31:21 UTC 2017
On Mon, Jan 23, 2017 at 05:12:11PM +0200, Pekka Paalanen wrote:
> On Thu, 12 Jan 2017 10:13:29 +1000
> Peter Hutterer <peter.hutterer at who-t.net> wrote:
>
> > This was already in the DTD but not supported by the scanner.
> >
> > The request/event parsing have an additional check that "since" cannot be <
> > interface version.
>
> Hi Peter,
>
> this comment threw me off for a moment. It's not the interface version
> (interface->version) you mean here, but a "counter" that remembers the
> previously processed message's since value (interface->since).
>
> > This is missing here. For requests/events we can rely on
> > the xml containing the required "since" attributes already. enums don't have
> > those, so our default version of 1 for an enum introduced in interface version
> > > 1 would trigger warnings about version running backwards.
>
> I don't quite understand this explanation. To me it should be more like
> the following.
>
> Requests and events need a non-decreasing since-value check, because
> their opcodes get determined by the element order in the XML. If
> since-value ever decreased, it would imply that some opcodes have
> changed, which would break the protocol.
>
> Entries for enums have no such ordering considerations.
ok, I completely misread what this code does, sorry. New patch coming up
in a bit.
Cheers,
Peter
> > This doesn't matter for the output, it's just warnings, hence why they're
> > skipped here.
> >
> > Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> > ---
> > Changes to v1:
> > - sync test-scanner data files with the new output
> >
> > src/scanner.c | 28 ++++++++++++++++++++++++----
> > tests/data/example-client.h | 9 +++++++++
> > tests/data/example-server.h | 9 +++++++++
> > tests/data/example.xml | 2 ++
> > 4 files changed, 44 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/scanner.c b/src/scanner.c
> > index 5d66fa4..9c92316 100644
> > --- a/src/scanner.c
> > +++ b/src/scanner.c
> > @@ -220,6 +220,7 @@ struct entry {
> > char *uppercase_name;
> > char *value;
> > char *summary;
> > + int since;
> > struct wl_list link;
> > };
> >
> > @@ -797,6 +798,8 @@ start_element(void *data, const char *element_name, const char **atts)
> > fail(&ctx->loc, "no entry name given");
> >
> > entry = create_entry(name, value);
> > + version = version_from_since(ctx, since);
> > + entry->since = version;
> >
> > if (summary)
> > entry->summary = xstrdup(summary);
> > @@ -1278,16 +1281,33 @@ emit_enumerations(struct interface *interface)
> > }
> > printf("enum %s_%s {\n", interface->name, e->name);
> > wl_list_for_each(entry, &e->entry_list, link) {
> > - if (entry->summary)
> > - printf("\t/**\n"
> > - "\t * %s\n"
> > - "\t */\n", entry->summary);
> > + if (entry->summary || entry->since > 1) {
> > + printf("\t/**\n");
> > + if (entry->summary)
> > + printf("\t * %s\n", entry->summary);
> > + if (entry->since > 1)
> > + printf("\t * @since %d\n", entry->since);
> > + printf("\t */\n");
> > + }
> > printf("\t%s_%s_%s = %s,\n",
> > interface->uppercase_name,
> > e->uppercase_name,
> > entry->uppercase_name, entry->value);
> > }
> > printf("};\n");
> > +
> > + wl_list_for_each(entry, &e->entry_list, link) {
> > + if (entry->since == 1)
> > + continue;
> > +
> > + printf("/**\n * @ingroup iface_%s\n */\n", interface->name);
> > + printf("#define %s_%s_%s_SINCE_VERSION %d\n",
> > + interface->uppercase_name,
> > + e->uppercase_name, entry->uppercase_name,
> > + entry->since);
> > +
> > + }
> > +
> > printf("#endif /* %s_%s_ENUM */\n\n",
> > interface->uppercase_name, e->uppercase_name);
> > }
> > diff --git a/tests/data/example-client.h b/tests/data/example-client.h
> > index 857aacf..b1d23f9 100644
> > --- a/tests/data/example-client.h
> > +++ b/tests/data/example-client.h
> > @@ -1778,7 +1778,16 @@ enum wl_data_offer_error {
> > * offer doesn't accept this request
> > */
> > WL_DATA_OFFER_ERROR_INVALID_OFFER = 3,
> > + /**
> > + * test 'since' attribute parsing
> > + * @since 3
> > + */
> > + WL_DATA_OFFER_ERROR_TEST_SINCE = 4,
> > };
> > +/**
> > + * @ingroup iface_wl_data_offer
> > + */
> > +#define WL_DATA_OFFER_ERROR_TEST_SINCE_SINCE_VERSION 3
> > #endif /* WL_DATA_OFFER_ERROR_ENUM */
> >
> > /**
> > diff --git a/tests/data/example-server.h b/tests/data/example-server.h
> > index f22f70f..f7190fd 100644
> > --- a/tests/data/example-server.h
> > +++ b/tests/data/example-server.h
> > @@ -1469,7 +1469,16 @@ enum wl_data_offer_error {
> > * offer doesn't accept this request
> > */
> > WL_DATA_OFFER_ERROR_INVALID_OFFER = 3,
> > + /**
> > + * test 'since' attribute parsing
> > + * @since 3
> > + */
> > + WL_DATA_OFFER_ERROR_TEST_SINCE = 4,
> > };
> > +/**
> > + * @ingroup iface_wl_data_offer
> > + */
> > +#define WL_DATA_OFFER_ERROR_TEST_SINCE_SINCE_VERSION 3
> > #endif /* WL_DATA_OFFER_ERROR_ENUM */
> >
> > /**
> > diff --git a/tests/data/example.xml b/tests/data/example.xml
> > index 22dcffd..0ad2577 100644
> > --- a/tests/data/example.xml
> > +++ b/tests/data/example.xml
> > @@ -427,6 +427,8 @@
> > summary="action argument has an invalid value"/>
> > <entry name="invalid_offer" value="3"
> > summary="offer doesn't accept this request"/>
> > + <entry name="test_since" value="4"
> > + summary="test 'since' attribute parsing" since="3"/>
> > </enum>
> >
> > <request name="accept">
>
> Having "since" already in the entry name had me read this twice. ;-)
>
> It would have been much better to make this addition to
> tests/data/small.xml instead, so that it can be left there, and you
> don't need to make diverting changes to example.xml which is kind of
> meant to be a copy of wayland.xml from some point in the past.
> Small.xml is meant to be a minimal protocol that should excercise all
> XML features (but likely lacks some).
>
> Then you can update wayland.xml with the proper use of the new
> attributes, and do not necessarily need to clutter that commit with
> updates to the test suite data. If you want to be thorough, there could
> be a final commit that just sync wayland.xml into example.xml again. I
> didn't intend to have example.xml updated every time wayland.xml
> changes, but in this case it seems appropriate.
>
> The scanner changes look good though and the output too.
>
>
> Thanks,
> pq
More information about the wayland-devel
mailing list