Re: [PATCH wayland 8/8] fixup! scanner: initialize .{method, event}_count via ARRAY_SIZE
On 3 July 2018 at 12:45, Pekka Paalanen wrote:
> On Thu, 14 Jun 2018 16:49:45 +0100
> Emil Velikov wrote:
>
>> ---
>> tests/data/example-code.c | 73
>> +
>> tests/data/small-code-core.c| 5 +--
>> tests/data/small-code.c | 5 +--
>> tests/data/small-private-code.c | 5 +--
>> 4 files changed, 46 insertions(+), 42 deletions(-)
>>
>> diff --git a/tests/data/example-code.c b/tests/data/example-code.c
>> index 2e1f73b..65d9651 100644
>> --- a/tests/data/example-code.c
>> +++ b/tests/data/example-code.c
>> @@ -146,6 +146,7 @@ static const struct wl_interface *types[] = {
>> [94] = _surface_interface,
>> };
>>
>> +#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
>> static const struct wl_message wl_display_requests[] = {
>> { .name = "sync", .signature = "n", .types = [8] },
>> { .name = "get_registry", .signature = "n", .types = [9] },
>> @@ -158,8 +159,8 @@ static const struct wl_message wl_display_events[] = {
>>
>> WL_EXPORT const struct wl_interface wl_display_interface = {
>> .name = "wl_display", .version = 1,
>> - .method_count = 2, .methods = wl_display_requests,
>> - .event_count = 2, .events = wl_display_events,
>> + .method_count = ARRAY_SIZE(wl_display_requests), .methods =
>> wl_display_requests,
>> + .event_count = ARRAY_SIZE(wl_display_events), .events =
>> wl_display_events,
>> };
>
> Hi,
>
> this change is not an obvious improvement to me. "method_count = 2" is
> pretty clear, that combined with "methods = foo" does not seem to
> leave anything to be desired.
>
> If this code was hand-written, then I would be cheering for ARRAY_SIZE
> for sure, but it's not. It all comes from a generator that gets the
> count right.
>
> I suppose we'd need an opinion from someone who is less familiar with
> Wayland C bindings.
>
Sure ARRAY_SIZE is used for getting thins "right" although it also
improves _readability_.
Pre-patch (series) it took me an embarrassing amount of time trying to
understand any of the magic created by the scanner.
Perhaps I'm just silly in trying to spare the next reader some time...
It's trivial change and basic coding practise.
HTH
Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland 8/8] fixup! scanner: initialize .{method, event}_count via ARRAY_SIZE
On July 3, 2018 12:45 PM, Pekka Paalanen wrote:
> On Thu, 14 Jun 2018 16:49:45 +0100
> Emil Velikov wrote:
>
> > ---
> > tests/data/example-code.c | 73
> > +
> > tests/data/small-code-core.c| 5 +--
> > tests/data/small-code.c | 5 +--
> > tests/data/small-private-code.c | 5 +--
> > 4 files changed, 46 insertions(+), 42 deletions(-)
> >
> > diff --git a/tests/data/example-code.c b/tests/data/example-code.c
> > index 2e1f73b..65d9651 100644
> > --- a/tests/data/example-code.c
> > +++ b/tests/data/example-code.c
> > @@ -146,6 +146,7 @@ static const struct wl_interface *types[] = {
> > [94] = _surface_interface,
> > };
> >
> > +#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
> > static const struct wl_message wl_display_requests[] = {
> > { .name = "sync", .signature = "n", .types = [8] },
> > { .name = "get_registry", .signature = "n", .types = [9] },
> > @@ -158,8 +159,8 @@ static const struct wl_message wl_display_events[] = {
> >
> > WL_EXPORT const struct wl_interface wl_display_interface = {
> > .name = "wl_display", .version = 1,
> > - .method_count = 2, .methods = wl_display_requests,
> > - .event_count = 2, .events = wl_display_events,
> > + .method_count = ARRAY_SIZE(wl_display_requests), .methods =
> > wl_display_requests,
> > + .event_count = ARRAY_SIZE(wl_display_events), .events =
> > wl_display_events,
> > };
>
> Hi,
>
> this change is not an obvious improvement to me. "method_count = 2" is
> pretty clear, that combined with "methods = foo" does not seem to
> leave anything to be desired.
>
> If this code was hand-written, then I would be cheering for ARRAY_SIZE
> for sure, but it's not. It all comes from a generator that gets the
> count right.
>
> I suppose we'd need an opinion from someone who is less familiar with
> Wayland C bindings.
Hi,
I'm not sure if I qualify, but I would agree with Pekka here. This generated
code isn't meant to be maintained or edited by humans. This code is read-only,
and is readable enough as is IMHO for the reasons Pekka wrote.
> Thanks,
> pq
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland 8/8] fixup! scanner: initialize .{method, event}_count via ARRAY_SIZE
On Thu, 14 Jun 2018 16:49:45 +0100
Emil Velikov wrote:
> ---
> tests/data/example-code.c | 73
> +
> tests/data/small-code-core.c| 5 +--
> tests/data/small-code.c | 5 +--
> tests/data/small-private-code.c | 5 +--
> 4 files changed, 46 insertions(+), 42 deletions(-)
>
> diff --git a/tests/data/example-code.c b/tests/data/example-code.c
> index 2e1f73b..65d9651 100644
> --- a/tests/data/example-code.c
> +++ b/tests/data/example-code.c
> @@ -146,6 +146,7 @@ static const struct wl_interface *types[] = {
> [94] = _surface_interface,
> };
>
> +#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
> static const struct wl_message wl_display_requests[] = {
> { .name = "sync", .signature = "n", .types = [8] },
> { .name = "get_registry", .signature = "n", .types = [9] },
> @@ -158,8 +159,8 @@ static const struct wl_message wl_display_events[] = {
>
> WL_EXPORT const struct wl_interface wl_display_interface = {
> .name = "wl_display", .version = 1,
> - .method_count = 2, .methods = wl_display_requests,
> - .event_count = 2, .events = wl_display_events,
> + .method_count = ARRAY_SIZE(wl_display_requests), .methods =
> wl_display_requests,
> + .event_count = ARRAY_SIZE(wl_display_events), .events =
> wl_display_events,
> };
Hi,
this change is not an obvious improvement to me. "method_count = 2" is
pretty clear, that combined with "methods = foo" does not seem to
leave anything to be desired.
If this code was hand-written, then I would be cheering for ARRAY_SIZE
for sure, but it's not. It all comes from a generator that gets the
count right.
I suppose we'd need an opinion from someone who is less familiar with
Wayland C bindings.
Thanks,
pq
pgpUYlUmFMBpD.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH wayland 8/8] fixup! scanner: initialize .{method, event}_count via ARRAY_SIZE
---
tests/data/example-code.c | 73 +
tests/data/small-code-core.c| 5 +--
tests/data/small-code.c | 5 +--
tests/data/small-private-code.c | 5 +--
4 files changed, 46 insertions(+), 42 deletions(-)
diff --git a/tests/data/example-code.c b/tests/data/example-code.c
index 2e1f73b..65d9651 100644
--- a/tests/data/example-code.c
+++ b/tests/data/example-code.c
@@ -146,6 +146,7 @@ static const struct wl_interface *types[] = {
[94] = _surface_interface,
};
+#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
static const struct wl_message wl_display_requests[] = {
{ .name = "sync", .signature = "n", .types = [8] },
{ .name = "get_registry", .signature = "n", .types = [9] },
@@ -158,8 +159,8 @@ static const struct wl_message wl_display_events[] = {
WL_EXPORT const struct wl_interface wl_display_interface = {
.name = "wl_display", .version = 1,
- .method_count = 2, .methods = wl_display_requests,
- .event_count = 2, .events = wl_display_events,
+ .method_count = ARRAY_SIZE(wl_display_requests), .methods =
wl_display_requests,
+ .event_count = ARRAY_SIZE(wl_display_events), .events =
wl_display_events,
};
static const struct wl_message wl_registry_requests[] = {
@@ -173,8 +174,8 @@ static const struct wl_message wl_registry_events[] = {
WL_EXPORT const struct wl_interface wl_registry_interface = {
.name = "wl_registry", .version = 1,
- .method_count = 1, .methods = wl_registry_requests,
- .event_count = 2, .events = wl_registry_events,
+ .method_count = ARRAY_SIZE(wl_registry_requests), .methods =
wl_registry_requests,
+ .event_count = ARRAY_SIZE(wl_registry_events), .events =
wl_registry_events,
};
static const struct wl_message wl_callback_events[] = {
@@ -183,7 +184,7 @@ static const struct wl_message wl_callback_events[] = {
WL_EXPORT const struct wl_interface wl_callback_interface = {
.name = "wl_callback", .version = 1,
- .event_count = 1, .events = wl_callback_events,
+ .event_count = ARRAY_SIZE(wl_callback_events), .events =
wl_callback_events,
};
static const struct wl_message wl_compositor_requests[] = {
@@ -193,7 +194,7 @@ static const struct wl_message wl_compositor_requests[] = {
WL_EXPORT const struct wl_interface wl_compositor_interface = {
.name = "wl_compositor", .version = 4,
- .method_count = 2, .methods = wl_compositor_requests,
+ .method_count = ARRAY_SIZE(wl_compositor_requests), .methods =
wl_compositor_requests,
};
static const struct wl_message wl_shm_pool_requests[] = {
@@ -204,7 +205,7 @@ static const struct wl_message wl_shm_pool_requests[] = {
WL_EXPORT const struct wl_interface wl_shm_pool_interface = {
.name = "wl_shm_pool", .version = 1,
- .method_count = 3, .methods = wl_shm_pool_requests,
+ .method_count = ARRAY_SIZE(wl_shm_pool_requests), .methods =
wl_shm_pool_requests,
};
static const struct wl_message wl_shm_requests[] = {
@@ -217,8 +218,8 @@ static const struct wl_message wl_shm_events[] = {
WL_EXPORT const struct wl_interface wl_shm_interface = {
.name = "wl_shm", .version = 1,
- .method_count = 1, .methods = wl_shm_requests,
- .event_count = 1, .events = wl_shm_events,
+ .method_count = ARRAY_SIZE(wl_shm_requests), .methods = wl_shm_requests,
+ .event_count = ARRAY_SIZE(wl_shm_events), .events = wl_shm_events,
};
static const struct wl_message wl_buffer_requests[] = {
@@ -231,8 +232,8 @@ static const struct wl_message wl_buffer_events[] = {
WL_EXPORT const struct wl_interface wl_buffer_interface = {
.name = "wl_buffer", .version = 1,
- .method_count = 1, .methods = wl_buffer_requests,
- .event_count = 1, .events = wl_buffer_events,
+ .method_count = ARRAY_SIZE(wl_buffer_requests), .methods =
wl_buffer_requests,
+ .event_count = ARRAY_SIZE(wl_buffer_events), .events = wl_buffer_events,
};
static const struct wl_message wl_data_offer_requests[] = {
@@ -251,8 +252,8 @@ static const struct wl_message wl_data_offer_events[] = {
WL_EXPORT const struct wl_interface wl_data_offer_interface = {
.name = "wl_data_offer", .version = 3,
- .method_count = 5, .methods = wl_data_offer_requests,
- .event_count = 3, .events = wl_data_offer_events,
+ .method_count = ARRAY_SIZE(wl_data_offer_requests), .methods =
wl_data_offer_requests,
+ .event_count = ARRAY_SIZE(wl_data_offer_events), .events =
wl_data_offer_events,
};
static const struct wl_message wl_data_source_requests[] = {
@@ -272,8 +273,8 @@ static const struct wl_message wl_data_source_events[] = {
WL_EXPORT const struct wl_interface wl_data_source_interface = {
.name = "wl_data_source", .version = 3,
- .method_count = 3, .methods = wl_data_source_requests,
- .event_count = 6, .events = wl_data_source_events,
+ .method_count =
