sim: fix a warning in dv-sockser.c at connected_p()

Message ID AS8P193MB1285CCA642054E2286C39AB7E4182@AS8P193MB1285.EURP193.PROD.OUTLOOK.COM
State New
Headers
Series sim: fix a warning in dv-sockser.c at connected_p() |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed

Commit Message

Bernd Edlinger May 2, 2024, 1:23 p.m. UTC
  In some O/S e.g. windows there is a warning here about the
unused variable flags which triggers a -Werror build failure.

Fix that by making the variable declaration optional.
---
 sim/common/dv-sockser.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
  

Comments

Tom Tromey May 2, 2024, 4:15 p.m. UTC | #1
>>>>> "Bernd" == Bernd Edlinger <bernd.edlinger@hotmail.de> writes:

Bernd> In some O/S e.g. windows there is a warning here about the
Bernd> unused variable flags which triggers a -Werror build failure.

Bernd> Fix that by making the variable declaration optional.

I think this would be better if this declaration were just removed and
then stuck into the condition, like:

  /* Set non-blocking i/o.  */
#if defined(F_GETFL) && defined(O_NONBLOCK)
  int flags = fcntl (sockser_fd, F_GETFL);
  ^^^ add this here

What do you think?

Tom
  
Bernd Edlinger May 3, 2024, 9 a.m. UTC | #2
On 5/2/24 18:15, Tom Tromey wrote:
>>>>>> "Bernd" == Bernd Edlinger <bernd.edlinger@hotmail.de> writes:
> 
> Bernd> In some O/S e.g. windows there is a warning here about the
> Bernd> unused variable flags which triggers a -Werror build failure.
> 
> Bernd> Fix that by making the variable declaration optional.
> 
> I think this would be better if this declaration were just removed and
> then stuck into the condition, like:
> 
>   /* Set non-blocking i/o.  */
> #if defined(F_GETFL) && defined(O_NONBLOCK)
>   int flags = fcntl (sockser_fd, F_GETFL);
>   ^^^ add this here
> 
> What do you think?
> 

This would not be compliant to C99, and it would be the first use of this
C11 feature in this file.  Therefore I did not want to go that way.
I did also consider adding braces { } around this whole block, but
actually I do think that guarding the declaration with one #if is also
acceptable and is looking not too ugly, since the function is not too complex.

Thanks
Bernd.

> Tom
  
Tom Tromey May 3, 2024, 3:35 p.m. UTC | #3
Bernd> This would not be compliant to C99, and it would be the first use of this
Bernd> C11 feature in this file.  Therefore I did not want to go that way.
Bernd> I did also consider adding braces { } around this whole block, but
Bernd> actually I do think that guarding the declaration with one #if is also
Bernd> acceptable and is looking not too ugly, since the function is not too complex.

According to sim/README-HACKING, the sim requires C11 now.

Tom
  
Pedro Alves May 3, 2024, 4:07 p.m. UTC | #4
On 2024-05-03 16:35, Tom Tromey wrote:
> Bernd> This would not be compliant to C99, and it would be the first use of this
> Bernd> C11 feature in this file.  Therefore I did not want to go that way.
> Bernd> I did also consider adding braces { } around this whole block, but
> Bernd> actually I do think that guarding the declaration with one #if is also
> Bernd> acceptable and is looking not too ugly, since the function is not too complex.
> 
> According to sim/README-HACKING, the sim requires C11 now.
> 

And declaring a variable in the middle of the block is a C99 feature, not C11, regardless.
  

Patch

diff --git a/sim/common/dv-sockser.c b/sim/common/dv-sockser.c
index db81233e25b..0cb46947885 100644
--- a/sim/common/dv-sockser.c
+++ b/sim/common/dv-sockser.c
@@ -218,11 +218,14 @@  sim_install_dv_sockser (SIM_DESC sd)
 static int
 connected_p (SIM_DESC sd)
 {
-  int numfds,flags;
+  int numfds;
   struct timeval tv;
   fd_set readfds;
   struct sockaddr sockaddr;
   socklen_t addrlen;
+#if defined(F_GETFL) && defined(O_NONBLOCK)
+  int flags;
+#endif
 
   if (sockser_listen_fd == -1)
     return 0;