From: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
To: Julia Lawall <julia.lawall@lip6.fr>,
"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
Subject: EDAC: remove unnecessary static in edac_fake_inject_write()
Date: Sun, 23 Jul 2017 00:42:38 -0500 [thread overview]
Message-ID: <6867696d-edd1-3d21-eb7e-af68e087750a@embeddedor.com> (raw)
Hi Julia,
On 07/23/2017 12:07 AM, Julia Lawall wrote:
>
>
> 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.
>
Yeah, I totally agree and, personally I consider that a bad coding
practice. But I think those kinds of issues should be addressed in a
different script.
> 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.
>
Yeah, I agree, but I don't see exactly where this argument is coming from ?
Notice that for both worse1 and worse2, what is returned is the address,
not the value of the static variable. At least that was my intention,
unless I maybe missing something ?
>>
>> 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.
>
Yeah, those cases should be analyzed manually and in case of doubt check
with the maintainers.
Thanks
next reply other threads:[~2017-07-23 5:42 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-23 5:42 Gustavo A. R. Silva [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:07 Julia Lawall
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=6867696d-edd1-3d21-eb7e-af68e087750a@embeddedor.com \
--to=gustavo@embeddedor.com \
--cc=bp@alien8.de \
--cc=cocci@systeme.lip6.fr \
--cc=garsilva@embeddedor.com \
--cc=julia.lawall@lip6.fr \
--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