[PATCH wayland 1/4] scanner: use c99 initializers for the wl_interface * array

[Pekka Paalanen]

On Wed, 30 Aug 2017 16:33:15 +0100
Emil Velikov <emil.l.velikov at gmail.com> wrote:

> On 30 August 2017 at 16:21, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> > From: Emil Velikov <emil.velikov at collabora.com>
> >
> > Allows us to remove the explicit NULL init, keeping the list shorter and
> > easier to read.
> >  

Hi Emil,

looks like all of these patches change the scanner output in some way,
but none of them fix the corresponding tests. You'll see it if you run
'make check'. We expect scanner patches to fix the expected test output
data in the same go, so that the tests keep on passing. The reason for
this is that we can see immediately from the patch itself how it
changes the scanner output without having to manually diff to verify.

It's easy to fix: run 'make check' and copy the actual output over the
expected output and git-add. Ensure 'make check' now succeeds. The
paths are in tests/scanner-test.sh.log.

Patch 1 sounds like a nice idea. It also makes it easier to see which
row a 'types + N' refers to. I'll wait to see the diff on the test
output though.

Patches 2, 3 and 4 miss the commit message explaining why you wrote
them. I guess it's the same: easier to read. I have to say the only
time I actually read those generated files is when I review scanner
changes.

About the DEFINE_INTERFACE(zxdg_surface_v6, 1) idea, I don't know what
the benefit would be. No-one writes it by hand, the generator handles
the repetitive writing anyway. IMHO, the code is more obvious without
such a macro.

> Note:
> As the trailing NULL entries will be omitted, the actual array size
> will be smaller as-is.
> Not sure if/how much that would matter as neither requests or events
> would reference those.
> 
> Still making it a fixed size array might be the wiser move.

Fixed size sounds good. It does look like libwayland wouldn't try to
access the trailing NULLs, but I'd feel safer keeping them.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20170915/fd89cced/attachment.sig>