From c739b975ad1cc52cae6b320376f88dba63ffdc85 Mon Sep 17 00:00:00 2001 From: Isaac Freund Date: Fri, 3 Mar 2023 22:02:33 +0100 Subject: [PATCH 1/4] scanner: allow element-type and enum on array args This allows an element type and an enum to optionally be specified on array args in protocol xml. If an enum is specified the element type is required and is subject to the same restrictions as a the type of a non-array argument with an enum specified. Signed-off-by: Isaac Freund --- protocol/wayland.dtd | 1 + src/scanner.c | 50 ++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/protocol/wayland.dtd b/protocol/wayland.dtd index ee062eea..6f888168 100644 --- a/protocol/wayland.dtd +++ b/protocol/wayland.dtd @@ -28,5 +28,6 @@ + diff --git a/src/scanner.c b/src/scanner.c index c512d231..45297e25 100644 --- a/src/scanner.c +++ b/src/scanner.c @@ -209,6 +209,12 @@ enum arg_type { FD }; +enum element_type { + ELEMENT_TYPE_NONE, + ELEMENT_TYPE_INT, + ELEMENT_TYPE_UNSIGNED, +}; + struct arg { char *name; enum arg_type type; @@ -217,6 +223,7 @@ struct arg { struct wl_list link; char *summary; char *enumeration_name; + enum element_type element_type; }; struct enumeration { @@ -716,6 +723,7 @@ start_element(void *data, const char *element_name, const char **atts) const char *allow_null = NULL; const char *enumeration_name = NULL; const char *bitfield = NULL; + const char *element_type = NULL; int i, version = 0; ctx->loc.line_number = XML_GetCurrentLineNumber(ctx->parser); @@ -743,6 +751,8 @@ start_element(void *data, const char *element_name, const char **atts) enumeration_name = atts[i + 1]; if (strcmp(atts[i], "bitfield") == 0) bitfield = atts[i + 1]; + if (strcmp(atts[i], "element-type") == 0) + element_type = atts[i + 1]; } ctx->character_data_length = 0; @@ -841,6 +851,20 @@ start_element(void *data, const char *element_name, const char **atts) else arg->enumeration_name = xstrdup(enumeration_name); + if (element_type == NULL) { + arg->element_type = ELEMENT_TYPE_NONE; + } else { + if (strcmp(element_type, "int") == 0) { + arg->element_type = ELEMENT_TYPE_INT; + } else if (strcmp(element_type, "uint") == 0) { + arg->element_type = ELEMENT_TYPE_UNSIGNED; + } else { + fail(&ctx->loc, + "invalid element type '%s', must be int or uint", + element_type); + } + } + if (summary) arg->summary = xstrdup(summary); @@ -953,15 +977,34 @@ verify_arguments(struct parse_context *ctx, struct arg *a; wl_list_for_each(a, &m->arg_list, link) { struct enumeration *e; + enum arg_type t; + + if (a->element_type != ELEMENT_TYPE_NONE && a->type != ARRAY) { + fail(&ctx->loc, + "only args of type array may specify an element-type"); + } + + switch (a->element_type) { + case ELEMENT_TYPE_NONE: + t = a->type; + break; + case ELEMENT_TYPE_INT: + t = INT; + break; + case ELEMENT_TYPE_UNSIGNED: + t = UNSIGNED; + break; + default: + abort(); + } if (!a->enumeration_name) continue; - e = find_enumeration(ctx->protocol, interface, a->enumeration_name); - switch (a->type) { + switch (t) { case INT: if (e && e->bitfield) fail(&ctx->loc, @@ -969,6 +1012,9 @@ verify_arguments(struct parse_context *ctx, break; case UNSIGNED: break; + case ARRAY: + fail(&ctx->loc, + "array arg must specify element-type if enum is specified"); default: fail(&ctx->loc, "enumeration-style argument has wrong type"); From 5cf0063e5c5fb57a85026e01edaaa92a5ed1c20b Mon Sep 17 00:00:00 2001 From: Isaac Freund Date: Thu, 9 Mar 2023 22:22:58 +0100 Subject: [PATCH 2/4] scanner: allow element-bits on array args This attribute is optional but required if the element-type attribute is specified. This attribute specifies the size of the array elements in bits to allow for better WAYLAND_DEBUG logging and scanner code generation. Signed-off-by: Isaac Freund --- protocol/wayland.dtd | 1 + src/scanner.c | 124 ++++++++++++++++++++++++++++--------------- 2 files changed, 82 insertions(+), 43 deletions(-) diff --git a/protocol/wayland.dtd b/protocol/wayland.dtd index 6f888168..6a9c55b5 100644 --- a/protocol/wayland.dtd +++ b/protocol/wayland.dtd @@ -29,5 +29,6 @@ + diff --git a/src/scanner.c b/src/scanner.c index 45297e25..a9d8e695 100644 --- a/src/scanner.c +++ b/src/scanner.c @@ -224,6 +224,7 @@ struct arg { char *summary; char *enumeration_name; enum element_type element_type; + int element_bits; }; struct enumeration { @@ -725,6 +726,7 @@ start_element(void *data, const char *element_name, const char **atts) const char *bitfield = NULL; const char *element_type = NULL; int i, version = 0; + int element_bits = 0; ctx->loc.line_number = XML_GetCurrentLineNumber(ctx->parser); for (i = 0; atts[i]; i += 2) { @@ -753,6 +755,21 @@ start_element(void *data, const char *element_name, const char **atts) bitfield = atts[i + 1]; if (strcmp(atts[i], "element-type") == 0) element_type = atts[i + 1]; + if (strcmp(atts[i], "element-bits") == 0) { + element_bits = strtouint(atts[i + 1]); + switch (element_bits) { + case 8: + case 16: + case 32: + /* Note: the wire protocol only guarantees 4 byte alignment + * of array data. Therefore we cannot allow 64 bit elements + * as they require 8 byte alignment on common targets. */ + break; + default: + fail(&ctx->loc, + "invalid element-bits value (%d)", atts[i + 1]); + } + } } ctx->character_data_length = 0; @@ -864,6 +881,7 @@ start_element(void *data, const char *element_name, const char **atts) element_type); } } + arg->element_bits = element_bits; if (summary) arg->summary = xstrdup(summary); @@ -966,6 +984,68 @@ find_enumeration(struct protocol *protocol, return NULL; } +static void +verify_argument_attributes(struct parse_context *ctx, + struct interface *interface, + struct arg *a) +{ + struct enumeration *e; + enum arg_type t; + + if (a->type == ARRAY) { + if ((a->element_type != ELEMENT_TYPE_NONE) != (a->element_bits != 0)) { + fail(&ctx->loc, + "if one of element-type or element-bits is specified " + "the other must be as well"); + } + } else { + if (a->element_type != ELEMENT_TYPE_NONE) { + fail(&ctx->loc, + "only args of type array may specify an element-type"); + } + if (a->element_bits != 0) { + fail(&ctx->loc, + "only args of type array may specify element-bits"); + } + } + + switch (a->element_type) { + case ELEMENT_TYPE_NONE: + t = a->type; + break; + case ELEMENT_TYPE_INT: + t = INT; + break; + case ELEMENT_TYPE_UNSIGNED: + t = UNSIGNED; + break; + default: + abort(); + } + + if (!a->enumeration_name) + return; + + e = find_enumeration(ctx->protocol, interface, + a->enumeration_name); + + switch (t) { + case INT: + if (e && e->bitfield) + fail(&ctx->loc, + "bitfield-style enum must only be referenced by uint"); + break; + case UNSIGNED: + break; + case ARRAY: + fail(&ctx->loc, + "array arg must specify element-type if enum is specified"); + default: + fail(&ctx->loc, + "enumeration-style argument has wrong type"); + } +} + static void verify_arguments(struct parse_context *ctx, struct interface *interface, @@ -976,49 +1056,7 @@ verify_arguments(struct parse_context *ctx, wl_list_for_each(m, messages, link) { struct arg *a; wl_list_for_each(a, &m->arg_list, link) { - struct enumeration *e; - enum arg_type t; - - if (a->element_type != ELEMENT_TYPE_NONE && a->type != ARRAY) { - fail(&ctx->loc, - "only args of type array may specify an element-type"); - } - - switch (a->element_type) { - case ELEMENT_TYPE_NONE: - t = a->type; - break; - case ELEMENT_TYPE_INT: - t = INT; - break; - case ELEMENT_TYPE_UNSIGNED: - t = UNSIGNED; - break; - default: - abort(); - } - - if (!a->enumeration_name) - continue; - - e = find_enumeration(ctx->protocol, interface, - a->enumeration_name); - - switch (t) { - case INT: - if (e && e->bitfield) - fail(&ctx->loc, - "bitfield-style enum must only be referenced by uint"); - break; - case UNSIGNED: - break; - case ARRAY: - fail(&ctx->loc, - "array arg must specify element-type if enum is specified"); - default: - fail(&ctx->loc, - "enumeration-style argument has wrong type"); - } + verify_argument_attributes(ctx, interface, a); } } From 823fcc6f54c2e19e53d1e731866536cfeeeec1af Mon Sep 17 00:00:00 2001 From: Isaac Freund Date: Mon, 13 Mar 2023 12:37:30 +0100 Subject: [PATCH 3/4] protocol: specify element type/bits of arrays The only array arg in the core protocol is the keys arg of the wl_keyboard.enter event. Signed-off-by: Isaac Freund --- protocol/wayland.xml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/protocol/wayland.xml b/protocol/wayland.xml index 10e039d6..fdc12423 100644 --- a/protocol/wayland.xml +++ b/protocol/wayland.xml @@ -2411,7 +2411,8 @@ - + From ce6f92fe7bfc8817d691de5c0a09d0868867b194 Mon Sep 17 00:00:00 2001 From: Isaac Freund Date: Fri, 3 Mar 2023 23:22:19 +0100 Subject: [PATCH 4/4] connection: print elements of arrays when possible If the array has element-type and element-bits specified in the xml use this to print the values of the array in WAYLAND_DEBUG=1 output. Signed-off-by: Isaac Freund --- src/connection.c | 110 ++++++++++++++++++++++++++++++++++++++++-- src/scanner.c | 12 ++++- src/wayland-private.h | 2 + src/wayland-util.h | 2 + 4 files changed, 122 insertions(+), 4 deletions(-) diff --git a/src/connection.c b/src/connection.c index ceaeac14..73c413d1 100644 --- a/src/connection.c +++ b/src/connection.c @@ -436,10 +436,28 @@ wl_connection_put_fd(struct wl_connection *connection, int32_t fd) return ring_buffer_put(&connection->fds_out, &fd, sizeof fd); } +static const char * +parse_array_signature(const char *signature, struct argument_details *details) +{ + details->type = *signature; + if (signature[1] == '[') { + details->element_type = signature[2]; + signature += 3; + while (*signature != ']') { + details->element_bits *= 10; + details->element_bits += (*signature) - '0'; + ++signature; + } + } + return signature + 1; +} + const char * get_next_argument(const char *signature, struct argument_details *details) { details->nullable = 0; + details->element_type = '\0'; + details->element_bits = 0; for(; *signature; ++signature) { switch(*signature) { case 'i': @@ -448,10 +466,11 @@ get_next_argument(const char *signature, struct argument_details *details) case 's': case 'o': case 'n': - case 'a': case 'h': details->type = *signature; return signature + 1; + case 'a': + return parse_array_signature(signature, details); case '?': details->nullable = 1; } @@ -472,9 +491,14 @@ arg_count_for_signature(const char *signature) case 's': case 'o': case 'n': - case 'a': case 'h': ++count; + break; + case 'a': + ++count; + if (signature[1] == '[') + while (*signature != ']') ++signature; + break; } } return count; @@ -1262,6 +1286,86 @@ wl_closure_queue(struct wl_closure *closure, struct wl_connection *connection) return result; } +static void +wl_closure_print_array(FILE *f, struct argument_details *arg, + struct wl_array *array) +{ + int8_t *i8; + int16_t *i16; + int32_t *i32; + uint8_t *u8; + uint16_t *u16; + uint32_t *u32; + + fprintf(f, "array["); + switch (arg->element_type) { + case 'i': + switch (arg->element_bits) { + case 8: + wl_array_for_each(i8, array) { + if (i8 == array->data) + fprintf(f, "%"PRId8, *i8); + else + fprintf(f, ",%"PRId8, *i8); + } + break; + case 16: + wl_array_for_each(i16, array) { + if (i16 == array->data) + fprintf(f, "%"PRId16, *i16); + else + fprintf(f, ",%"PRId16, *i16); + } + break; + case 32: + wl_array_for_each(i32, array) { + if (i32 == array->data) + fprintf(f, "%"PRId32, *i32); + else + fprintf(f, ",%"PRId32, *i32); + } + break; + default: + abort(); + } + break; + case 'u': + switch (arg->element_bits) { + case 8: + wl_array_for_each(u8, array) { + if (u8 == array->data) + fprintf(f, "%"PRIu8, *u8); + else + fprintf(f, ",%"PRIu8, *u8); + } + break; + case 16: + wl_array_for_each(u16, array) { + if (u16 == array->data) + fprintf(f, "%"PRIu16, *u16); + else + fprintf(f, ",%"PRIu16, *u16); + } + break; + case 32: + wl_array_for_each(u32, array) { + if (u32 == array->data) + fprintf(f, "%"PRIu32, *u32); + else + fprintf(f, ",%"PRIu32, *u32); + } + break; + default: + abort(); + } + break; + case '\0': + fprintf(f, "size: %zu", array->size); + break; + } + fprintf(f, "]"); +} + void wl_closure_print(struct wl_closure *closure, struct wl_object *target, int send, int discarded, uint32_t (*n_parse)(union wl_argument *arg)) @@ -1345,7 +1449,7 @@ wl_closure_print(struct wl_closure *closure, struct wl_object *target, fprintf(f, "nil"); break; case 'a': - fprintf(f, "array[%zu]", closure->args[i].a->size); + wl_closure_print_array(f, &arg, closure->args[i].a); break; case 'h': fprintf(f, "fd %d", closure->args[i].h); diff --git a/src/scanner.c b/src/scanner.c index a9d8e695..6617beb0 100644 --- a/src/scanner.c +++ b/src/scanner.c @@ -1877,7 +1877,17 @@ emit_messages(const char *name, struct wl_list *message_list, printf("o"); break; case ARRAY: - printf("a"); + switch (a->element_type) { + case ELEMENT_TYPE_NONE: + printf("a"); + break; + case ELEMENT_TYPE_INT: + printf("a[i%d]", a->element_bits); + break; + case ELEMENT_TYPE_UNSIGNED: + printf("a[u%d]", a->element_bits); + break; + } break; case FD: printf("h"); diff --git a/src/wayland-private.h b/src/wayland-private.h index 9274f1b8..cf6dfdb4 100644 --- a/src/wayland-private.h +++ b/src/wayland-private.h @@ -151,6 +151,8 @@ struct wl_closure { struct argument_details { char type; int nullable; + char element_type; + int element_bits; }; const char * diff --git a/src/wayland-util.h b/src/wayland-util.h index b4cdcfad..acf0d091 100644 --- a/src/wayland-util.h +++ b/src/wayland-util.h @@ -119,6 +119,8 @@ struct wl_object; * * `a`: array * * `h`: fd * * `?`: following argument (`o` or `s`) is nullable + * * `[i32]`: the preceding array argument has 32 bit signed integer elements + * * `[u8]`: the preceding array argument has 8 bit unsigned integer elements * * While demarshaling primitive arguments is straightforward, when demarshaling * messages containing `object` or `new_id` arguments, the protocol