[v2] Avoid RMW of flags2 outside lock (BZ #27842)

Message ID DB6PR0801MB18796AC2183729447CDEA9C583D49@DB6PR0801MB1879.eurprd08.prod.outlook.com
State Committed
Commit f107b7b30d815f191181617e2c14f21e6af4ddcf
Headers
Series [v2] Avoid RMW of flags2 outside lock (BZ #27842) |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit success Build for i686

Commit Message

Wilco Dijkstra May 23, 2022, 3:47 p.m. UTC
  v2: remove flags2 update altogether since it isn't needed.

Remove an unconditional RMW on flags2 in flockfile - we don't need to change
_IO_FLAGS2_NEED_LOCK since it isn't used in flockfile or funlockfile.
This fixes BZ #27842.

---
  

Comments

Adhemerval Zanella June 8, 2022, 4:53 p.m. UTC | #1
On 23/05/2022 12:47, Wilco Dijkstra wrote:
> v2: remove flags2 update altogether since it isn't needed.
> 
> Remove an unconditional RMW on flags2 in flockfile - we don't need to change
> _IO_FLAGS2_NEED_LOCK since it isn't used in flockfile or funlockfile.
> This fixes BZ #27842.

I think it should be ok, although POSIX states that:

  "All functions that reference (FILE *) objects, except those with names ending 
   in _unlocked, shall behave as if they use flockfile() and funlockfile() 
   internally to obtain ownership of these (FILE *) objects."

Skipping the locks on single-thread case should not issue any visible
side-effects (since flockfile does take the lock regardless).

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> 
> ---
> 
> diff --git a/stdio-common/flockfile.c b/stdio-common/flockfile.c
> index a5decb450f8d477e3105d02661282afeab58f88b..49f72c69ab7a4ccca6f21d443c6fc3a0eca76376 100644
> --- a/stdio-common/flockfile.c
> +++ b/stdio-common/flockfile.c
> @@ -22,7 +22,6 @@
>  void
>  __flockfile (FILE *stream)
>  {
> -  stream->_flags2 |= _IO_FLAGS2_NEED_LOCK;
>    _IO_lock_lock (*stream->_lock);
>  }
>  weak_alias (__flockfile, flockfile);
  

Patch

diff --git a/stdio-common/flockfile.c b/stdio-common/flockfile.c
index a5decb450f8d477e3105d02661282afeab58f88b..49f72c69ab7a4ccca6f21d443c6fc3a0eca76376 100644
--- a/stdio-common/flockfile.c
+++ b/stdio-common/flockfile.c
@@ -22,7 +22,6 @@ 
 void
 __flockfile (FILE *stream)
 {
-  stream->_flags2 |= _IO_FLAGS2_NEED_LOCK;
   _IO_lock_lock (*stream->_lock);
 }
 weak_alias (__flockfile, flockfile);