[PATCH 1/1] Exit status of fc -e is the wrong way around

[Ingo Karkat]

Hello Bash developers!

Configuration Information [Automatically generated, do not change]:
Machine: x86_64
OS: linux-gnu
Compiler: gcc
Compilation CFLAGS:  -DPROGRAM='bash' -DCONF_HOSTTYPE='x86_64' 
-DCONF_OSTYPE='linux-gnu' -DCONF_MACHTYPE='x86_64-unknown-linux-gnu' 
-DCONF_VENDOR='unknown' -DLOCALEDIR='/usr/local/share/locale' -DPACKAGE='bash' 
-DSHELL -DHAVE_CONFIG_H   -I.  -I. -I./include -I./lib   -g -O2
uname output: Linux nashi 3.19.0-61-generic #69~14.04.1-Ubuntu SMP Thu Jun 9 
09:09:13 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
Machine Type: x86_64-unknown-linux-gnu

Bash Version: 4.3
Patch Level: 46
Release Status: release
Also seen in prior versions, at least down to 4.2.46(1)-release

Description:

I'm trying to edit and recall a previous command with Bash's built-in "fc -e 
vi" command, but somehow the exit status always indicates failure (1) on 
successful edit, even if that editing is a no-op. The recalled command itself 
executes just fine, just $? is wrong.
Originally reported (and more background) here: 
http://unix.stackexchange.com/questions/292847/exit-status-of-fc-e-is-the-wrong-way-around

Repeat-By:

    $ uname
    Linux
    $ fc -e true
    uname
    Linux
    $ echo $?
    1
    $ # Would have expected 0 here: successful re-invocation

Fix:

This is my first dive into Bash's sources; patch follows and is also attached 
to this mail. Please let me know if you need anything else. As a longtime Bash 
user, I'm very happy to contribute to my favorite shell!

-- regards, ingo

From 1cf392a401c67c2f8437f2da459dfcf0f675dc55 Mon Sep 17 00:00:00 2001
From: Ingo Karkat <sw...@ingo-karkat.de>
Date: Thu, 30 Jun 2016 12:30:59 +0200
Subject: [PATCH 1/1] Exit status of fc -e is the wrong way around

fc_execute_file() delegates to _evalfile(), which only returns the result of 
the file's execution if FEVAL_BUILTIN is set (exemplified by source_file()). If 
unset, an indication of whether the file exists is returned instead 
(exemplified by maybe_execute_file(), which is used for the .bash_profile, 
.bash_login, .profile optional init chain).

According to the manual (and common sense), fc -e editor should return the 
recalled command's success. For that, the FEVAL_BUILTIN flag needs to be set.
---
 builtins/evalfile.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtins/evalfile.c b/builtins/evalfile.c
index 058d99d..e5c118b 100644
--- a/builtins/evalfile.c
+++ b/builtins/evalfile.c
@@ -331,7 +331,7 @@ fc_execute_file (filename)
 
   /* We want these commands to show up in the history list if
      remember_on_history is set. */
-  flags = FEVAL_ENOENTOK|FEVAL_HISTORY|FEVAL_REGFILE;
+  flags = FEVAL_ENOENTOK|FEVAL_BUILTIN|FEVAL_HISTORY|FEVAL_REGFILE;
   return (_evalfile (filename, flags));
 }
 #endif /* HISTORY */
-- 
1.9.1

-- 
  -- Ingo Karkat --  /^-- /^-- /^-- /^-- /^-- http://ingo-karkat.de/ --

From 1cf392a401c67c2f8437f2da459dfcf0f675dc55 Mon Sep 17 00:00:00 2001
From: Ingo Karkat <sw...@ingo-karkat.de>
Date: Thu, 30 Jun 2016 12:30:59 +0200
Subject: [PATCH 1/1] Exit status of fc -e is the wrong way around

fc_execute_file() delegates to _evalfile(), which only returns the result of the file's execution if FEVAL_BUILTIN is set (exemplified by source_file()). If unset, an indication of whether the file exists is returned instead (exemplified by maybe_execute_file(), which is used for the .bash_profile, .bash_login, .profile optional init chain).

According to the manual (and common sense), fc -e editor should return the recalled command's success. For that, the FEVAL_BUILTIN flag needs to be set.
---
 builtins/evalfile.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtins/evalfile.c b/builtins/evalfile.c
index 058d99d..e5c118b 100644
--- a/builtins/evalfile.c
+++ b/builtins/evalfile.c
@@ -331,7 +331,7 @@ fc_execute_file (filename)
 
   /* We want these commands to show up in the history list if
      remember_on_history is set. */
-  flags = FEVAL_ENOENTOK|FEVAL_HISTORY|FEVAL_REGFILE;
+  flags = FEVAL_ENOENTOK|FEVAL_BUILTIN|FEVAL_HISTORY|FEVAL_REGFILE;
   return (_evalfile (filename, flags));
 }
 #endif /* HISTORY */
-- 
1.9.1

signature.asc
Description: OpenPGP digital signature