On 07/23/2011 05:50 PM, Denys Vlasenko wrote:
On Wednesday 13 July 2011 15:43, Dennis Groenen wrote:
On 07/13/2011 03:13 PM, Laurent Bercot wrote:
   Specialized filesystems such as jffs2 or yaffs2 still can, however.
Good point, but a lot of, for example closed, embedded/mobile devices
require a lot of hacking around to get something custom (like a
different fs) running.
   Why shouldn't it be? Either you want to reduce the number of writes
to the filesystem, or you don't. Delayed writing, which is configured
at the filesystem layer, accomplishes this for every application without
the need for patching. Applications can always fsync() when they need
synchronous writes.
True, but generally those writes from other applications are going to
happen anyway. E.g. if I save a file in application X, it will need to
be written out, and be written out completely. For BusyBox's history,
this isn't the case. I don't see why a file has to be written out after
each appended line here. Vi doesn't write out a file after each new line
either, does it?
sh can crash.
sh can be killed by a signal.
sh can be run in parallel from two terminals, and saving at exit creates
unfairness: whoever exits last overwrites the history of other guys.

The issues you've described are very valid, and are somewhat inevitable when using volatile memory as a buffer. One needs to take these issues into account when deciding whether to use a history buffer or not.

Nevertheless, I am not firmly against saving only on exit.
Actually, I think that's the option you'd like.
So, (1) it would largely solve your problem
and (2) it would also be welcomed by people who like
more bash-like behavior wrt history saving.

Can you code up a patch which does this?

I don't think I'm skilled enough to create a 'true' in-memory buffer to temporarily store the history, i.e. one that doesn't use a regular file in e.g. /tmp as the buffer. I'm assuming here that you didn't like the way how I implemented saving history on exit in my original patch.

On a side note, using a regular file in /tmp does solve all the issues you've posed before. If sh gets killed or crashes, the regular file persists and will be written out upon the next (clean) shell exit. You'll only lose (some) history if /tmp gets cleared during a shell session. Also, as libbb/lineedit.c isn't touched, BusyBox' original code for dealing with parallel terminals is preserved, thus this sort of buffer isn't less unfair than vanilla BusyBox.

--Dennis

P.s. I've attached a revised version of my original patch for those interested in it. It has better error handling and is more secure. Do note that it still uses a regular file in /tmp as the history buffer.
>From c1b4608e82299d128101eae3b027565d0a4274bb Mon Sep 17 00:00:00 2001
From: Dennis Groenen <[email protected]>
Date: Wed, 27 Jul 2011 15:26:45 +0200
Subject: [PATCH] ash: add support for history buffer

---
 shell/ash.c |   84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 81 insertions(+), 3 deletions(-)

diff --git a/shell/ash.c b/shell/ash.c
index d48cd01..fda72a2 100644
--- a/shell/ash.c
+++ b/shell/ash.c
@@ -184,6 +184,24 @@
 //config:	  This option recreates the prompt string from the environment
 //config:	  variable each time it is displayed.
 //config:
+//config:config ASH_HIST_BUFFER
+//config:	bool "History buffer"
+//config:	default n
+//config:	depends on ASH && FEATURE_EDITING_SAVEHISTORY
+//config:	help
+//config:	  Allows you to set a temporary location for .ash_history.
+//config:	  History is saved to this custom location, and written out to
+//config:	  its default location (~/.ash_history) upon shell exit.
+//config:	  Useful to prevent wear on flash-based storage devices.
+//config:
+//config:config ASH_HIST_BUFFER_PATH
+//config:	string "History buffer location"
+//config:	default "/tmp"
+//config:	depends on ASH && ASH_HIST_BUFFER
+//config:	help
+//config:	  Directory which will be used to save the shell history until
+//config:	  ash is exited.
+//config:
 
 //applet:IF_ASH(APPLET(ash, BB_DIR_BIN, BB_SUID_DROP))
 //applet:IF_FEATURE_SH_IS_ASH(APPLET_ODDNAME(sh, ash, BB_DIR_BIN, BB_SUID_DROP, sh))
@@ -12888,6 +12906,14 @@ exitshell(void)
 	char *p;
 	int status;
 
+#if ENABLE_ASH_HIST_BUFFER
+	const char *tmphistory = lookupvar("HISTFILE");
+	const char *storedhistory = lookupvar("STOREDHISTFILE");
+
+	if (storedhistory) /* is NULL when setting up the history buffer failed; check this before copying */
+		copy_file(tmphistory, storedhistory, FILEUTILS_FORCE | FILEUTILS_DEREFERENCE | FILEUTILS_PRESERVE_STATUS);
+#endif
+
 	status = exitstatus;
 	TRACE(("pid %d, exitshell(%d)\n", getpid(), status));
 	if (setjmp(loc.loc)) {
@@ -13151,9 +13177,61 @@ int ash_main(int argc UNUSED_PARAM, char **argv)
 		if (!hp) {
 			hp = lookupvar("HOME");
 			if (hp) {
-				char *defhp = concat_path_file(hp, ".ash_history");
-				setvar("HISTFILE", defhp, 0);
-				free(defhp);
+#if ENABLE_ASH_HIST_BUFFER
+				char *tmppath;
+				char *tmphistory;
+				char *storedhistory;
+				const char *user = lookupvar("USER");
+
+				tmppath = concat_path_file(CONFIG_ASH_HIST_BUFFER_PATH, user);
+				tmphistory = concat_path_file(tmppath, ".ash_history");
+				storedhistory = concat_path_file(hp, ".ash_history");
+
+				if (access(tmphistory, R_OK | W_OK) == -1) {
+					if (access(CONFIG_ASH_HIST_BUFFER_PATH, R_OK == -1)) {
+						bb_simple_perror_msg("could not access history buffer path");
+						goto bail;
+					}
+					if (bb_make_directory(tmppath, 0700, FILEUTILS_RECUR)) {
+						/* bb_make_directory is noisy, no need for an additional error message here */
+						goto bail;
+					}
+					if (access(storedhistory, R_OK) == -1) {
+						creat(tmphistory, 0644);
+					} else {
+						copy_file(storedhistory, tmphistory, FILEUTILS_FORCE | FILEUTILS_DEREFERENCE | FILEUTILS_PRESERVE_STATUS);
+					}
+				} else { /* (security) checks before reusing existing temporary history file */
+					struct stat stat_tmphistory;
+					lstat(tmphistory, &stat_tmphistory);
+					if (!S_ISREG(stat_tmphistory.st_mode)) {
+						errno = 0;
+						bb_simple_perror_msg("history buffer is not a regular file");
+						goto bail;
+					}
+					if (stat_tmphistory.st_uid != geteuid() || stat_tmphistory.st_gid != getegid()) {
+						errno = 0;
+						bb_simple_perror_msg("history buffer is not exclusive to the shell user");   
+						goto bail;
+					}
+				}
+				setvar("STOREDHISTFILE", storedhistory, 0);
+				goto out;
+
+ bail:
+				errno = 0;
+				bb_simple_perror_msg("shell history will not be saved");
+				tmphistory = xasprintf("/dev/null");
+ out:
+				setvar("HISTFILE", tmphistory, 0);
+				free(storedhistory);
+				free(tmphistory);
+				free(tmppath);
+#else
+ 				char *defhp = concat_path_file(hp, ".ash_history");
+ 				setvar("HISTFILE", defhp, 0);
+ 				free(defhp);
+#endif
 			}
 		}
 	}
-- 
1.7.6


_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox

Reply via email to