Re: [LTP] [PATCH 4/7] docparse: Implement ARRAY_SIZE()

Cyril Hrubis
2021-11-01T13:17:56Z

Hi!
> > Adds a special handlingn for ARRAY_SIZE() macro.
> >
> > If we stumble upon ARRAY_SIZE() in the tst_test structure we try to
> > lookup the array and count its members.
>
> Note that this is the type of thing Sparse should be able to handle
> well. As it should evaluate this to a constant. Although there is the
> risk it would be too clever and eliminate some expressions (for
> example).

That's another possible problem, the parser may end up too smart, but
that is something we will see once we try.

Hi!
> > + if (!strcmp(token, "#include")) {
>
> Are spaces removed after the '#'?
>
> e.g.
>
> #if X
> # include<Y>
> #endif

Good catch, will fix that in v2 and add a test.

> > diff --git a/docparse/tests/array_size01.c b/docparse/tests/array_size01.c
> > new file mode 100644
> > index 000000000..8a0b9252b
> > --- /dev/null
> > +++ b/docparse/tests/array_size01.c
> > @@ -0,0 +1,5 @@
> > +static int variants = {1};
>
> I can see how this might work, but looks odd.

This supposed to be static int variants[] = {1};

The thing is that we do not even have a parser here, it's an extractor
and we depend on the C code being correct. It does compile by a compiler
before we even attempt to extract the metadata. So it will happily
process input that is not correct C.

I will fix the test in v2 as well.

> > diff --git a/docparse/tests/array_size02.c b/docparse/tests/array_size02.c
> > new file mode 100644
> > index 000000000..5c7d4471e
> > --- /dev/null
> > +++ b/docparse/tests/array_size02.c
> > @@ -0,0 +1,5 @@
> > +static int variants = {{1}, {2}, {3}};
>
> Should this be variants[]?

Indeed, it should be also struct foo { int val } type and not int. Will
fix a swell.

--
Cyril Hrubis
chrubis@suse.cz