Linux EDAC development
 help / color / mirror / Atom feed
From: Julia Lawall <julia.lawall@lip6.fr>
To: "Gustavo A. R. Silva" <garsilva@embeddedor.com>
Cc: Borislav Petkov <bp@alien8.de>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	linux-edac@vger.kernel.org, linux-kernel@vger.kernel.org,
	cocci@systeme.lip6.fr,
	"Gustavo A. R. Silva" <gustavo@embeddedor.com>
Subject: EDAC: remove unnecessary static in edac_fake_inject_write()
Date: Sun, 23 Jul 2017 07:07:59 +0200 (CEST)	[thread overview]
Message-ID: <alpine.DEB.2.20.1707230654200.2068@hadrien> (raw)

On Sat, 22 Jul 2017, Gustavo A. R. Silva wrote:

> Hi Julia, Borislav,
>
> On 07/22/2017 11:22 AM, Gustavo A. R. Silva wrote:
> > Hi all,
> >
> > On 07/22/2017 01:36 AM, Borislav Petkov wrote:
> > > On Fri, Jul 21, 2017 at 10:08:12PM +0200, Julia Lawall wrote:
> > > > Someone pointed out that the rule is probably not OK when the address of
> > > > the static variable is taken, because then it is likely being used as
> > > > permanent storage.
> > >
> > > Makes sense to me.
> > >
> > > > An improved rule is:
> > >
> > > Do you think it is worth having it in scripts/coccinelle/ ?
> > >
> > > I don't think Gustavo would mind putting it there :)
> > >
> >
> > Absolutely, I'd be glad to help out. :)
> >
>
> I've been working on this issue today and, in my opinion, this script is even
> better:
>
> @bad exists@
> position p;
> identifier x;
> expression e;
> type T;
> @@
>
> static T x@p;
> ... when != x = e
> x = <+...x...+>
>
> @worse1 exists@
> position p;
> identifier x;
> type T;
> @@
>
> static T x@p;
> ...
> return &x;
>
> @worse2 exists@
> position p;
> identifier x;
> type T;
> @@
>
> static T *x@p;
> ...
> return x;
>
> @@
> identifier x;
> expression e;
> type T;
> position p != {bad.p,worse1.p,worse2.p};
> @@
>
> -static
>   T x@p;
>   ... when != x
>       when strict
> ?x = e;
>
> It ignores all the cases in which the address of the static variable is
> returned to the caller function.

I don't understand why you want to restrict the address of a variable case
to returns.  Storing the address in a field of a structure that has a
lifetime beyond the function body is a problem as well.

On the other hand returning the value stored in a static variable is not a
problem.  That value exists independently of the variable that contains
it.  The variable that conains it doesn't need to live on in any way.

>
> Also, there are some cases in which the maintainer can argue something like
> the following:
>
> https://lkml.org/lkml/2017/7/19/1381
>
> but that depends on the particular conditions in which the code is intended to
> be executed.
>
> What do you think?

The preserving values argument is not relevant.  The rule checks that the
value is never used.  DMA accesses should involve taking an address, which
we now disallow.  It seems likely that anything large would have its
address taken too, but one could check manually for that.  spgen provides
a section where you can describe such issues.

julia

> Thank you
> --
> Gustavo A. R. Silva
>
---
To unsubscribe from this list: send the line "unsubscribe linux-edac" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

             reply	other threads:[~2017-07-23  5:07 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-23  5:07 Julia Lawall [this message]
  -- strict thread matches above, loose matches on Subject: below --
2017-07-24 10:34 EDAC: remove unnecessary static in edac_fake_inject_write() Borislav Petkov
2017-07-23  6:25 Gustavo A. R. Silva
2017-07-23  5:53 Julia Lawall
2017-07-23  5:42 Gustavo A. R. Silva
2017-07-23  2:31 Gustavo A. R. Silva
2017-07-22 16:22 Gustavo A. R. Silva
2017-07-22  6:39 Julia Lawall
2017-07-22  6:36 Borislav Petkov
2017-07-21 20:08 Julia Lawall
2017-07-17  8:39 Borislav Petkov
2017-07-04 21:44 Gustavo A. R. Silva

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.20.1707230654200.2068@hadrien \
    --to=julia.lawall@lip6.fr \
    --cc=bp@alien8.de \
    --cc=cocci@systeme.lip6.fr \
    --cc=garsilva@embeddedor.com \
    --cc=gustavo@embeddedor.com \
    --cc=linux-edac@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mchehab@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox