diff mbox series

libiberty: writeargv: Simplify function error mode.

Message ID CAHyHGCnV8p_Qs0AZhBYKzUy+inMGCH6hE3ZfVnp1Q3o+ZoC_ng@mail.gmail.com
State Accepted
Headers
Series libiberty: writeargv: Simplify function error mode. |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Costas Argyris June 5, 2023, 2:37 p.m. UTC 
  writeargv can be simplified by getting rid of the error exit mode
that was only relevant many years ago when the function used
to open the file descriptor internally.

Comments

Jeff Law June 6, 2023, 3:12 a.m. UTC | #1 
On 6/5/23 08:37, Costas Argyris via Gcc-patches wrote:
> writeargv can be simplified by getting rid of the error exit mode
> that was only relevant many years ago when the function used
> to open the file descriptor internally.
[ ... ]
Thanks.  I've pushed this to the trunk.

You could (as a follow-up) simplify it even further.  There's no need 
for the status variable as far as I can tell.  You could just have the 
final return be "return 0;" instead of "return status;".

Jeff
Costas Argyris June 6, 2023, 8:44 a.m. UTC | #2 
You are right, this is also a remnant of the old function design
that I completely missed.    Here is the follow-up patch for that.

Thanks for pointing it out.

Costas

On Tue, 6 Jun 2023 at 04:12, Jeff Law <jeffreyalaw@gmail.com> wrote:

>
>
> On 6/5/23 08:37, Costas Argyris via Gcc-patches wrote:
> > writeargv can be simplified by getting rid of the error exit mode
> > that was only relevant many years ago when the function used
> > to open the file descriptor internally.
> [ ... ]
> Thanks.  I've pushed this to the trunk.
>
> You could (as a follow-up) simplify it even further.  There's no need
> for the status variable as far as I can tell.  You could just have the
> final return be "return 0;" instead of "return status;".
>
> Jeff
>
Jeff Law June 7, 2023, 2:52 a.m. UTC | #3 
On 6/6/23 02:44, Costas Argyris wrote:
> You are right, this is also a remnant of the old function design
> that I completely missed.    Here is the follow-up patch for that.
> 
> Thanks for pointing it out.
> 
> Costas
> 
> On Tue, 6 Jun 2023 at 04:12, Jeff Law <jeffreyalaw@gmail.com 
> <mailto:jeffreyalaw@gmail.com>> wrote:
> 
> 
> 
>     On 6/5/23 08:37, Costas Argyris via Gcc-patches wrote:
>      > writeargv can be simplified by getting rid of the error exit mode
>      > that was only relevant many years ago when the function used
>      > to open the file descriptor internally.
>     [ ... ]
>     Thanks.  I've pushed this to the trunk.
> 
>     You could (as a follow-up) simplify it even further.  There's no need
>     for the status variable as far as I can tell.  You could just have the
>     final return be "return 0;" instead of "return status;".
> 
>     Jeff
> 
> 
> 0001-libiberty-writeargv-Remove-unnecessary-status-variab.patch
> 
>  From 13fdfea60eeac64e028315392614b955e998487d Mon Sep 17 00:00:00 2001
> From: Costas Argyris<costas.argyris@gmail.com>
> Date: Tue, 6 Jun 2023 09:15:48 +0100
> Subject: [PATCH] libiberty: writeargv: Remove unnecessary status variable.
> 
> Follow-up from 4d1e4ce986f pointed out by jlaw.
> 
> Signed-off-by: Costas Argyris<costas.argyris@gmail.com
> ---
>   libiberty/argv.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
Thanks.  I created a ChangeLog entry and committed this change to the trunk.

Jeff
diff mbox series

Patch

From 1271552baee5561fa61652f4ca7673c9667e4f8f Mon Sep 17 00:00:00 2001
From: Costas Argyris <costas.argyris@gmail.com>
Date: Mon, 5 Jun 2023 15:02:06 +0100
Subject: [PATCH] libiberty: writeargv: Simplify function error mode.

The goto-based error mode was based on a previous version
of the function where it was responsible for opening the
file, so it had to close it upon any exit:

https://inbox.sourceware.org/gcc-patches/20070417200340.GM9017@sparrowhawk.codesourcery.com/

(thanks pinskia)

This is no longer the case though since now the function
takes the file descriptor as input, so the exit mode on
error can be just a simple return 1 statement.

Signed-off-by: Costas Argyris <costas.argyris@gmail.com>
---
 libiberty/argv.c | 29 +++++++++--------------------
 1 file changed, 9 insertions(+), 20 deletions(-)

diff --git a/libiberty/argv.c b/libiberty/argv.c
index a95a10e14ff..1a18b4d8866 100644
--- a/libiberty/argv.c
+++ b/libiberty/argv.c
@@ -289,8 +289,8 @@  char **buildargv (const char *input)
 @deftypefn Extension int writeargv (char * const *@var{argv}, FILE *@var{file})
 
 Write each member of ARGV, handling all necessary quoting, to the file
-named by FILE, separated by whitespace.  Return 0 on success, non-zero
-if an error occurred while writing to FILE.
+associated with FILE, separated by whitespace.  Return 0 on success,
+non-zero if an error occurred while writing to FILE.
 
 @end deftypefn
 
@@ -314,36 +314,25 @@  writeargv (char * const *argv, FILE *f)
 
           if (ISSPACE(c) || c == '\\' || c == '\'' || c == '"')
             if (EOF == fputc ('\\', f))
-              {
-                status = 1;
-                goto done;
-              }
+              return 1;
 
           if (EOF == fputc (c, f))
-            {
-              status = 1;
-              goto done;
-            }
+            return 1;
+	  
           arg++;
         }
 
       /* Write out a pair of quotes for an empty argument.  */
       if (arg == *argv)
-	if (EOF == fputs ("\"\"", f))
-	  {
-	    status = 1;
-	    goto done;
-	  }
+        if (EOF == fputs ("\"\"", f))
+          return 1;
 
       if (EOF == fputc ('\n', f))
-        {
-          status = 1;
-          goto done;
-        }
+        return 1;
+      
       argv++;
     }
 
- done:
   return status;
 }
 
-- 
2.30.2