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

[Cyril Hrubis <chrubis@suse.cz>]

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

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp