[PATCH 0/2] Two small F_SETFL changes

[PATCH 0/2] Two small F_SETFL changes

[Tom Tromey]

I was looking at the F_SETFL / SIGIO handling code in gdbserver
recently, and I noticed a couple of oddities.

This series fixes both of them.  I wouldn't expect these patches to
really affect anything in practice.

Signed-off-by: Tom Tromey <tromey@adacore.com>
---
Tom Tromey (2):
      Don't use F_SETFL in gdbreplay
      Use F_SETFL after F_SETOWN

 gdbserver/gdbreplay.cc    | 3 ---
 gdbserver/remote-utils.cc | 6 ++----
 2 files changed, 2 insertions(+), 7 deletions(-)
---
base-commit: f8ebb8db3bd5bcfaeaa42f803fe15300e65196ee
change-id: 20260717-f-setfl-1556b2b5ed46

Best regards,
--  
Tom Tromey <tromey@adacore.com>

[PATCH 1/2] Don't use F_SETFL in gdbreplay

[Tom Tromey]

There's no need to call fcntl with F_SETFL in gdbreplay, as gdbreplay
does not use or need SIGIO.
---
 gdbserver/gdbreplay.cc | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/gdbserver/gdbreplay.cc b/gdbserver/gdbreplay.cc
index 4426dc316ec..ba03bf64b67 100644
--- a/gdbserver/gdbreplay.cc
+++ b/gdbserver/gdbreplay.cc
@@ -255,9 +255,6 @@ remote_open (const char *name)
 #endif
     }
 
-#if defined(F_SETFL) && defined (FASYNC)
-  fcntl (remote_desc_in, F_SETFL, FASYNC);
-#endif
   remote_desc_out = remote_desc_in;
 }
 

-- 
2.55.0

Re: [PATCH 1/2] Don't use F_SETFL in gdbreplay

[Simon Marchi]

On 7/17/26 1:07 PM, Tom Tromey wrote:
> There's no need to call fcntl with F_SETFL in gdbreplay, as gdbreplay
> does not use or need SIGIO.
> ---
>  gdbserver/gdbreplay.cc | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/gdbserver/gdbreplay.cc b/gdbserver/gdbreplay.cc
> index 4426dc316ec..ba03bf64b67 100644
> --- a/gdbserver/gdbreplay.cc
> +++ b/gdbserver/gdbreplay.cc
> @@ -255,9 +255,6 @@ remote_open (const char *name)
>  #endif
>      }
>  
> -#if defined(F_SETFL) && defined (FASYNC)
> -  fcntl (remote_desc_in, F_SETFL, FASYNC);
> -#endif
>    remote_desc_out = remote_desc_in;


LGTM, but also it looks like you could drop:

#if HAVE_FCNTL_H
#include <fcntl.h>
#endif

Approved-By: Simon Marchi <simon.marchi@efficios.com>

Simon

Re: [PATCH 1/2] Don't use F_SETFL in gdbreplay

[Tom Tromey]

Simon> LGTM, but also it looks like you could drop:

Simon> #if HAVE_FCNTL_H
Simon> #include <fcntl.h>
Simon> #endif

I did this.

Tom

[PATCH 2/2] Use F_SETFL after F_SETOWN

[Tom Tromey]

This changes enable_async_notification to use F_SETFL after F_SETOWN.
This order more correct because it ensures that the owning process is
set before the request to enable SIGIO.
---
 gdbserver/remote-utils.cc | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/gdbserver/remote-utils.cc b/gdbserver/remote-utils.cc
index ffde4956776..e92adf903ce 100644
--- a/gdbserver/remote-utils.cc
+++ b/gdbserver/remote-utils.cc
@@ -128,13 +128,11 @@ static void
 enable_async_notification (int fd)
 {
 #if defined(F_SETFL) && defined (FASYNC)
-  int save_fcntl_flags;
-
-  save_fcntl_flags = fcntl (fd, F_GETFL, 0);
-  fcntl (fd, F_SETFL, save_fcntl_flags | FASYNC);
 #if defined (F_SETOWN)
   fcntl (fd, F_SETOWN, getpid ());
 #endif
+  int save_fcntl_flags = fcntl (fd, F_GETFL, 0);
+  fcntl (fd, F_SETFL, save_fcntl_flags | FASYNC);
 #endif
 }
 

-- 
2.55.0

Re: [PATCH 2/2] Use F_SETFL after F_SETOWN

[Simon Marchi]

On 7/17/26 1:07 PM, Tom Tromey wrote:
> This changes enable_async_notification to use F_SETFL after F_SETOWN.
> This order more correct because it ensures that the owning process is

Missing "is".

> set before the request to enable SIGIO.
> ---
>  gdbserver/remote-utils.cc | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/gdbserver/remote-utils.cc b/gdbserver/remote-utils.cc
> index ffde4956776..e92adf903ce 100644
> --- a/gdbserver/remote-utils.cc
> +++ b/gdbserver/remote-utils.cc
> @@ -128,13 +128,11 @@ static void
>  enable_async_notification (int fd)
>  {
>  #if defined(F_SETFL) && defined (FASYNC)
> -  int save_fcntl_flags;
> -
> -  save_fcntl_flags = fcntl (fd, F_GETFL, 0);
> -  fcntl (fd, F_SETFL, save_fcntl_flags | FASYNC);
>  #if defined (F_SETOWN)
>    fcntl (fd, F_SETOWN, getpid ());
>  #endif
> +  int save_fcntl_flags = fcntl (fd, F_GETFL, 0);
> +  fcntl (fd, F_SETFL, save_fcntl_flags | FASYNC);

While touching these lines, could you maybe indent the inner #if and
#endif? Like:

#if defined(F_SETFL) && defined (FASYNC)
#  if defined (F_SETOWN)
  fcntl (fd, F_SETOWN, getpid ());
#  endif
  int save_fcntl_flags = fcntl (fd, F_GETFL, 0);
  fcntl (fd, F_SETFL, save_fcntl_flags | FASYNC);
#endif

Approved-By: Simon Marchi <simon.marchi@efficios.com>

Simon

Re: [PATCH 2/2] Use F_SETFL after F_SETOWN

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> While touching these lines, could you maybe indent the inner #if and
Simon> #endif?

Sure thing.

Tom