[PATCH v2 wayland 3/5] scanner: support "since" attribute for enum entries
Pekka Paalanen
ppaalanen at gmail.com
Mon Jan 23 15:12:11 UTC 2017
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.
>
> 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 801 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20170123/2a49ddfa/attachment-0001.sig>
More information about the wayland-devel
mailing list