All of lore.kernel.org
 help / color / mirror / Atom feed
From: Luca Fancellu <Luca.Fancellu@arm.com>
To: Julien Grall <julien@xen.org>
Cc: Xen-devel <xen-devel@lists.xenproject.org>,
	Bertrand Marquis <Bertrand.Marquis@arm.com>,
	Wei Chen <Wei.Chen@arm.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	George Dunlap <george.dunlap@citrix.com>,
	Jan Beulich <jbeulich@suse.com>,
	Stefano Stabellini <sstabellini@kernel.org>, Wei Liu <wl@xen.org>
Subject: Re: [PATCH] docs/misra: Add instructions for cppcheck
Date: Fri, 24 Jun 2022 13:34:14 +0000	[thread overview]
Message-ID: <CA8DFF26-3D7F-4CDA-9EDC-E173203B2A51@arm.com> (raw)
In-Reply-To: <88bd7017-e2b3-59f3-a68a-25db9e53136d@xen.org>



> On 24 Jun 2022, at 13:17, Julien Grall <julien@xen.org> wrote:
> 
> 
> 
> On 24/06/2022 13:01, Luca Fancellu wrote:
>> Hi Julien,
> 
> Hi Luca,
> 
>>>> +First recommendation is to use exactly the same version in this page and provide
>>>> +the same option to the build system, so that every Xen developer can reproduce
>>>> +the same findings.
>>> 
>>> I am not sure I agree. I think it is good that each developper use their own version (so long it is supported), so they may be able to find issues that may not appear with 2.7.
>> Yes I understand, but as Bertrand says, other version of this tool doesn’t work quite well. 
> 
> I have replied to this on Bertrand e-mail.
> 
> 
>> I agree that everyone should use their own version, but for the sake of reproducibility
>> of the findings, I think we should have a common ground.
> 
> I will reply to this below.
> 
>> The community can however propose from time to time to bump the version as long as we can say it works (maybe
>> crossing the reports between cppcheck, eclair, other proprietary tools).
> 
> This would mean we should de-support 2.7 which sounds wrong if it worked before.

Sure, I guess that as long as we don’t see regressions from version X to X+1 we are fine with versions >= X.

>>> 
>>>> +
>>>> +Install cppcheck in the system
>>> 
>>> NIT: s/in/on/ I think.
>> Sure will fix
>>> 
>>>> +==============================
>>>> +
>>>> +Cppcheck can be retrieved from the github repository or by downloading the
>>>> +tarball, the version tested so far is the 2.7:
>>>> +
>>>> + - https://github.com/danmar/cppcheck/tree/2.7
>>>> + - https://github.com/danmar/cppcheck/archive/2.7.tar.gz
>>>> +
>>>> +To compile and install it, here the complete command line:
>>>> +
>>>> +make MATCHCOMPILER=yes \
>>>> + FILESDIR=/usr/share/cppcheck \
>>>> + CFGDIR=/usr/share/cppcheck/cfg \
>>>> + HAVE_RULES=yes \
>>>> + CXXFLAGS="-O2 -DNDEBUG -Wall -Wno-sign-compare -Wno-unused-function" \
>>>> + install
>>> 
>>> Let me start that I am not convinced that our documentation should explain how to build cppcheck.
>>> 
>>> But if that's desire, then I think you ought to explain why we need to update CXXFLAGS (I would expect cppcheck to build everywhere without specifying additional flags).
>> Yes you are right, this is the recommended command line for building as in https://github.com/danmar/cppcheck/blob/main/readme.md section GNU make, I can add the source.
> 
> I think we should remove the command line and tell the user to read the cppcheck README.md.

Ok sounds good to me

> 
>> My intention when writing this page was to have a common ground between Xen developers, so that if one day someone came up with a fix for something, we are able to reproduce
>> the finding all together.
> Well, if someone find a fix you want to check against all versions not the one that warns. Otherwise, you can end up in a situation where you silence cppcheck 2.10 (just making up a version) but then introduce a warning in cppcheck 2.7.
> 
> To me this is no different than other software used to build Xen. We don't tell the user that they should always build with GCC x.y.z. Instead, we provide a minimum version. This has multiple benefits:
> 1) The user doesn't need to rebuild the software and can use the one provided by the distributions
> 2) Different versions find different (most of the time) valid bugs. So we are getting towards a better codebase.
> 

Ok I see your point, instead of saying “we use version X.Y, I will say >=X.Y”, your comment on Bertrand’s reply is on this line.

I would keep the section about compiling cppcheck since many recent distro doesn’t provide cppcheck >=2.7 yet (and 2.8 is broken),
If you agree with it.

For this one:
> 
> Thanks for the information. How about writing something like:
> 
> "
> The minimum version required for cppcheck is 2.7. Note that at the time of writing (June 2022), the version 2.8 is known to be broken [1].
> "
> 
> [1] https://sourceforge.net/p/cppcheck/discussion/general/thread/bfc3ab6c41/?limit=25
> 
> 

Sure, I can add it and rephrase that section.

Cheers,
Luca



  reply	other threads:[~2022-06-24 13:35 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-24 10:53 [PATCH] docs/misra: Add instructions for cppcheck Luca Fancellu
2022-06-24 11:20 ` Julien Grall
2022-06-24 11:40   ` Bertrand Marquis
2022-06-24 12:08     ` Julien Grall
2022-06-24 12:18       ` Bertrand Marquis
2022-06-24 12:22         ` Julien Grall
2022-06-24 12:26           ` Bertrand Marquis
2022-06-24 12:01   ` Luca Fancellu
2022-06-24 12:17     ` Julien Grall
2022-06-24 13:34       ` Luca Fancellu [this message]
2022-06-24 17:25         ` Julien Grall
2022-06-28 15:23           ` Luca Fancellu
2022-06-29 10:16             ` Julien Grall
2022-06-29 10:17               ` Luca Fancellu

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=CA8DFF26-3D7F-4CDA-9EDC-E173203B2A51@arm.com \
    --to=luca.fancellu@arm.com \
    --cc=Bertrand.Marquis@arm.com \
    --cc=Wei.Chen@arm.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=george.dunlap@citrix.com \
    --cc=jbeulich@suse.com \
    --cc=julien@xen.org \
    --cc=sstabellini@kernel.org \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.