diff mbox series

checkpatch: enable volatile warning

Message ID 20210310110451.10885-1-dharmik.thakkar@arm.com (mailing list archive)
State Rejected, archived
Delegated to: Thomas Monjalon
Headers
Series checkpatch: enable volatile warning |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/travis-robot success travis build: passed
ci/github-robot success github build: passed
ci/iol-abi-testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-mellanox-Functional success Functional Testing PASS
ci/iol-testing success Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS

Commit Message

Dharmik Thakkar March 10, 2021, 11:04 a.m. UTC 
  Enable volatile considered harmful warning since use of volatile
is suspect.

Suggested-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
---
 devtools/checkpatches.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Thomas Monjalon March 10, 2021, 11:10 a.m. UTC | #1 
10/03/2021 12:04, Dharmik Thakkar:
> Enable volatile considered harmful warning since use of volatile
> is suspect.
> 
> Suggested-by: Stephen Hemminger <stephen@networkplumber.org>
> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>

git grep -w volatile | wc -l
	1796

How much is it suspect?
Stephen Hemminger March 10, 2021, 3:10 p.m. UTC | #2 
On Wed, 10 Mar 2021 12:10:01 +0100
Thomas Monjalon <thomas@monjalon.net> wrote:

> 10/03/2021 12:04, Dharmik Thakkar:
> > Enable volatile considered harmful warning since use of volatile
> > is suspect.
> > 
> > Suggested-by: Stephen Hemminger <stephen@networkplumber.org>
> > Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>  
> 
> git grep -w volatile | wc -l
> 	1796
> 
> How much is it suspect?
> 
> 

Many seem to be unsafe.
   testpmd: uses flags values in unsafe manner
      it also uses volatile when accessing hardware registers

   test-alarm is expecting that alarm() is a signal
       (it is not)

   test-atomic is ok
   test-barrier is doing barriers and not using __atomic

   drivers use volatile to mark hardware registers.

It is still true volatile is not enough are not weak memory model.
Dharmik Thakkar Aug. 16, 2021, 1:28 p.m. UTC | #3 
Hi,

Apologies for the delayed response!

> On Mar 10, 2021, at 9:10 AM, Stephen Hemminger <stephen@networkplumber.org> wrote:
> 
> On Wed, 10 Mar 2021 12:10:01 +0100
> Thomas Monjalon <thomas@monjalon.net> wrote:
> 
>> 10/03/2021 12:04, Dharmik Thakkar:
>>> Enable volatile considered harmful warning since use of volatile
>>> is suspect.
>>> 
>>> Suggested-by: Stephen Hemminger <stephen@networkplumber.org>
>>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
>>> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>  
>> 
>> git grep -w volatile | wc -l
>> 	1796
>> 
>> How much is it suspect?
>> 
>> 
> 
> Many seem to be unsafe.
>   testpmd: uses flags values in unsafe manner
>      it also uses volatile when accessing hardware registers
> 
>   test-alarm is expecting that alarm() is a signal
>       (it is not)
> 
>   test-atomic is ok
>   test-barrier is doing barriers and not using __atomic
> 
>   drivers use volatile to mark hardware registers.
> 
> It is still true volatile is not enough are not weak memory model.

Agree, and this warning should help avoid such unsafe / incorrect use of volatile for future patches.
diff mbox series

Patch

diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
index 78a408ef9823..92f1984454b0 100755
--- a/devtools/checkpatches.sh
+++ b/devtools/checkpatches.sh
@@ -29,7 +29,7 @@  options="$options --max-line-length=$length"
 options="$options --show-types"
 options="$options --ignore=LINUX_VERSION_CODE,ENOSYS,\
 FILE_PATH_CHANGES,MAINTAINERS_STYLE,SPDX_LICENSE_TAG,\
-VOLATILE,PREFER_PACKED,PREFER_ALIGNED,PREFER_PRINTF,\
+PREFER_PACKED,PREFER_ALIGNED,PREFER_PRINTF,\
 PREFER_KERNEL_TYPES,PREFER_FALLTHROUGH,BIT_MACRO,CONST_STRUCT,\
 SPLIT_STRING,LONG_LINE_STRING,C99_COMMENT_TOLERANCE,\
 LINE_SPACING,PARENTHESIS_ALIGNMENT,NETWORKING_BLOCK_COMMENT_STYLE,\