linux-kernel - Re: [PATCH v3] coccinelle: semantic patch to check for inappropriate do

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c4ada2f2-19b0-91ef-ddf3-a1999f4209ea@linux.alibaba.com>
Date:   Sat, 11 Jan 2020 13:06:34 +0800
From:   Wen Yang <wenyang@...ux.alibaba.com>
To:     Markus Elfring <Markus.Elfring@....de>, cocci@...teme.lip6.fr,
        kernel-janitors@...r.kernel.org
Cc:     linux-kernel@...r.kernel.org,
        Gilles Muller <Gilles.Muller@...6.fr>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Julia Lawall <Julia.Lawall@...6.fr>,
        Masahiro Yamada <yamada.masahiro@...ionext.com>,
        Matthias Männich <maennich@...gle.com>,
        Michal Marek <michal.lkml@...kovi.net>,
        Nicolas Palix <nicolas.palix@...g.fr>,
        Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH v3] coccinelle: semantic patch to check for inappropriate
 do_div() calls



On 2020/1/11 12:35 上午, Markus Elfring wrote:
>> +@...tialize:python@
> …
>> +def filter_out_safe_constants(str):
> …
>> +def construct_warnings(str, suggested_fun):
> 
> * I suggest once more to adjust the dependency specifications for the usage
>    of these functions by SmPL rules.
> 

Most of the functions here are for all operation modes.


> * Can the local variable “msg” be omitted?
> 
> 
>> +coccilib.org.print_todo(p[0], construct_warnings("div64_ul"))
> 
> I suggest again to move the prefix “div64_” into the string literal
> of the function implementation.
> 

“div64_ul” indicates the function name we recommend.
As shown in the patch:

+def construct_warnings(suggested_fun):
+    msg="WARNING: do_div() does a 64-by-32 division, please consider 
using %s instead."
+    return  msg % suggested_fun
...
+coccilib.org.print_todo(p[0], construct_warnings("div64_ul"))

If we delete the prefix "div64_", it may reduce readability.

> 
> The SmPL code for two disjunctions could become shorter.
> 

You may suggest to modify it as follows:
+@@
+*do_div(f, \( l \| ul \| ul64 \| sl64 \) );

We agree with Julia:
I don't se any point to this.  The code matched will be the same in both
cases.  The original code is quite readable, without the ugly \( etc.

--
Regards，
Wen

> Regards,
> Markus
>