On Thu, 2018-08-23 at 18:13 -0400, Julia Lawall wrote:
>
> On Thu, 23 Aug 2018, Kees Cook wrote:
>
> > On Thu, Aug 23, 2018 at 3:00 PM, Julia Lawall <julia.law...@lip6.fr> wrote:
> > >
> > >
> > > On Thu, 23 Aug 2018, Kees Cook wrote:
> > >
> > > > On Thu, Aug 23, 2018 at 2:48 PM, Joe Perches <j...@perches.com> wrote:
> > > > > Forwarding a question about coccinelle and isomorphisms from Kees Cook
> > > > >
> > > > > ---------- Forwarded message ----------
> > > > > From: Kees Cook <keesc...@chromium.org>
> > > > > To: "Gustavo A. R. Silva" <gust...@embeddedor.com>
> > > > > Cc: Alessandro Zummo <a.zu...@towertech.it>, Alexandre Belloni
> > > > > <alexandre.bell...@bootlin.com>, Maxime Ripard
> > > > > <maxime.rip...@bootlin.com>, Chen-Yu Tsai <w...@csie.org>,
> > > > > linux-...@vger.kernel.org, linux-arm-kernel
> > > > > <linux-arm-ker...@lists.infradead.org>, LKML
> > > > > <linux-ker...@vger.kernel.org>
> > > > > Bcc:
> > > > > Date: Thu, 23 Aug 2018 13:56:35 -0700
> > > > > Subject: Re: [PATCH] rtc: sun6i: Use struct_size() in kzalloc()
> > > > > On Thu, Aug 23, 2018 at 11:51 AM, Gustavo A. R. Silva
> > > > > <gust...@embeddedor.com> wrote:
> > > > > > One of the more common cases of allocation size calculations is
> > > > > > finding
> > > > > > the size of a structure that has a zero-sized array at the end,
> > > > > > along
> > > > > > with memory for some number of elements for that array. For example:
> > > > > >
> > > > > > struct foo {
> > > > > > int stuff;
> > > > > > void *entry[];
> > > > > > };
> > > > > >
> > > > > > instance = kzalloc(sizeof(struct foo) + sizeof(void *) * count,
> > > > > > GFP_KERNEL);
> > > > > >
> > > > > > Instead of leaving these open-coded and prone to type mistakes, we
> > > > > > can
> > > > > > now use the new struct_size() helper:
> > > > > >
> > > > > > instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL);
> > > > > >
> > > > > > Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
> > > > > > ---
> > > > > > drivers/rtc/rtc-sun6i.c | 3 +--
> > > > > > 1 file changed, 1 insertion(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/rtc/rtc-sun6i.c b/drivers/rtc/rtc-sun6i.c
> > > > > > index 2cd5a7b..fe07310 100644
> > > > > > --- a/drivers/rtc/rtc-sun6i.c
> > > > > > +++ b/drivers/rtc/rtc-sun6i.c
> > > > > > @@ -199,8 +199,7 @@ static void __init sun6i_rtc_clk_init(struct
> > > > > > device_node *node)
> > > > > > if (!rtc)
> > > > > > return;
> > > > > >
> > > > > > - clk_data = kzalloc(sizeof(*clk_data) +
> > > > > > (sizeof(*clk_data->hws) * 2),
> > > > > > - GFP_KERNEL);
> > > > > > + clk_data = kzalloc(struct_size(clk_data, hws, 2),
> > > > > > GFP_KERNEL);
> > > > > > if (!clk_data) {
> > > > > > kfree(rtc);
> > > > > > return;
> > > > >
> > > > > This looks like entirely correct to me, but I'm surprised the
> > > > > Coccinelle script didn't discover this. I guess the isomorphisms don't
> > > > > cover the parenthesis?
> > > >
> > > > I had this:
> > > >
> > > > @@
> > > > identifier alloc =~
> > > > "kmalloc|kzalloc|kmalloc_node|kzalloc_node|vmalloc|vzalloc|kvmalloc|kvzalloc";
> > > > identifier VAR, ELEMENT;
> > > > expression COUNT;
> > > > @@
> > > >
> > > > - alloc(sizeof(*VAR) + COUNT * sizeof(*VAR->ELEMENT)
> > > > + alloc(struct_size(VAR, ELEMENT, COUNT)
> > > > , ...)
> > > >
> > > > But I needed to explicitly change the rule to:
> > > >
> > > > (
> > > > - alloc(sizeof(*VAR) + COUNT * sizeof(*VAR->ELEMENT)
> > > > + alloc(struct_size(VAR, ELEMENT, COUNT)
> > > > , ...)
> > > > >
> > > >
> > > > - alloc(sizeof(*VAR) + (COUNT * sizeof(*VAR->ELEMENT))
> > > > + alloc(struct_size(VAR, ELEMENT, COUNT)
> > > > , ...)
> > > > )
> > > >
> > > > to add the ()s. I would expect arithmetic commutative expressions to
> > > > match? But I had to add parens?
> > >
> > > Isomorphisms don't add parens. They only remove them. If they added
> > > them, you would end up with the possibility of having them everywhere, in
> > > all permutations, which would be slow and useless.
> >
> > Would a rule for:
> >
> > a + (b * c)
> >
> > match:
> >
> > a + b * c
>
> I would say yes. Basically it removes the parentheses but doesn't reparse
> the code, so it doesn't redo the associativity. Since * has higher
> precedence than +, then both will be matched. On the other hand, if you
> put:
>
> (a + b) * c
>
> It will consider a pattern with the parentheses removed, but that pattern
> won't match anything, because real trees that consist of a + b * c are
> parsed in a different way.
spatch 1.0.4 doesn't seem to:
$ spatch --version
spatch version 1.0.4 with Python support and with PCRE support
$ cat match_mul.cocci
@@
expression a, b, c;
int d;
@@
* d = a * b + c;
$ cat test_mul.c
int a, b, c, d;
void foo(void)
{
d = (a * b) + c;
d = a * b + c;
}
$ spatch -sp-file match_mul.cocci test_mul.c
init_defs_builtins: /usr/lib/coccinelle/standard.h
HANDLING: test_mul.c
diff =
--- test_mul.c
+++ /tmp/cocci-output-22607-ba6f76-test_mul.c
@@ -3,5 +3,4 @@ int a, b, c, d;
void foo(void)
{
d = (a * b) + c;
- d = a * b + c;
}
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
