linux-kernel - Re: [PATCH] Documentation/memory-barriers: fix wrong comment in example

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: <alpine.DEB.2.10.1602222006200.16296@hxeon>
Date:	Mon, 22 Feb 2016 20:08:15 +0900 (KST)
From:	SeongJae Park <sj38.park@...il.com>
To:	David Howells <dhowells@...hat.com>
cc:	SeongJae Park <sj38.park@...il.com>,
	Paul McKenney <paulmck@...ux.vnet.ibm.com>,
	Jonathan Corbet <corbet@....net>,
	linux-doc <linux-doc@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] Documentation/memory-barriers: fix wrong comment in
 example



On Mon, 22 Feb 2016, David Howells wrote:

> SeongJae Park <sj38.park@...il.com> wrote:
>
>>   a = 0;
>>   /* Code that does not store to variable a. */
>> + does_not_change_a();
>>   a = 0;
>
> Since it's not actually code that's meant to be executed, you could make it:
>
> 	a = 0;
> 	... code that does not store to variable a ...
> 	a = 0;

I selected Paul's third option because the function could be noop (In this
case, it doesn't break the original meaning) and it makes the code looks
complete.
However, your suggestion looks much better than the comment, too.

So, I am attaching a patch that applying your suggestion below.


=============================== >3 ====================================
>From f7b5677790771599f418f1d95536935be971ae86 Mon Sep 17 00:00:00 2001
From: SeongJae Park <sj38.park@...il.com>
Date: Mon, 22 Feb 2016 19:26:18 +0900
Subject: [PATCH] Documentation/memory-barriers: polish compiler store omit
  example

Comments of examples about compiler store omit in memory-barriers.txt is
about code that could be possible at that point.  However, someone could
interpret the comment as an explanation about below line.  This commit
exploits the intent more explicitly by changing the comment to be seems
like a possible code rather than explanation about below line.

Signed-off-by: SeongJae Park <sj38.park@...il.com>
---
  Documentation/memory-barriers.txt | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/memory-barriers.txt 
b/Documentation/memory-barriers.txt
index 904ee42..dc66351 100644
--- a/Documentation/memory-barriers.txt
+++ b/Documentation/memory-barriers.txt
@@ -1459,7 +1459,7 @@ of optimizations:
       the following:

         a = 0;
-       /* Code that does not store to variable a. */
+       ... Code that does not store to variable a ...
         a = 0;

       The compiler sees that the value of variable 'a' is already zero, so
@@ -1471,7 +1471,7 @@ of optimizations:
       wrong guess:

         WRITE_ONCE(a, 0);
-       /* Code that does not store to variable a. */
+       ... Code that does not store to variable a ...
         WRITE_ONCE(a, 0);

   (*) The compiler is within its rights to reorder memory accesses unless
--
1.9.1



>
> David
>