Re: [RFA] (cli/cli-cmds.c) ARI fix: Avoid assignment inside if statement

[Joel Brobecker <brobecker at adacore dot com>]

> 2012-12-20  Pierre Muller  <muller@sourceware.org>
> 
>         ARI fixes: Assignment within if rule.
>         * cli/cli-cmds.c (shell_escape): Do not set variable value inside
>         if statement.
>         (edit_command): Likewise.

Thanks for doing this, Pierre!

Aside from the meta-discussion on the rule itself, does anyone
see a problem with this patch? I see it as a clear improvement,
so I would like to approve it.

One tiny comment below...

> Index: src/gdb/cli/cli-cmds.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/cli/cli-cmds.c,v
> retrieving revision 1.143
> diff -u -p -r1.143 cli-cmds.c
> --- src/gdb/cli/cli-cmds.c      18 Dec 2012 19:27:35 -0000      1.143
> +++ src/gdb/cli/cli-cmds.c      20 Dec 2012 16:10:27 -0000
> @@ -724,11 +724,13 @@ shell_escape (char *arg, int from_tty)
>  #else /* Can fork.  */
>    int status, pid;
> 
> -  if ((pid = vfork ()) == 0)
> +  pid = vfork ();
> +  if (pid == 0)
>      {
>        const char *p, *user_shell;
> 
> -      if ((user_shell = (char *) getenv ("SHELL")) == NULL)
> +      user_shell = (char *) getenv ("SHELL");
> +      if (user_shell == NULL)
>         user_shell = "/bin/sh";
> 
>        /* Get the name of the shell for arg0.  */
> @@ -837,7 +839,8 @@ edit_command (char *arg, int from_tty)
>          error (_("No line number known for %s."), arg);
>      }
> 
> -  if ((editor = (char *) getenv ("EDITOR")) == NULL)
> +  editor = (char *) getenv ("EDITOR");

According to POSIX, getenv returns char *, so I do not think
that the cast is necessary. Let's try without and see what happens...

> +  if (editor == NULL)
>        editor = "/bin/ex";
> 
>    /* If we don't already know the full absolute file name of the

-- 
Joel