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

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

[Wilco Dijkstra]

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.

---

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);

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

[Adhemerval Zanella]

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);