From 57abf9e947fa3f7d69f7adb97023b299916ee63c Mon Sep 17 00:00:00 2001 From: Denis Vlasenko Date: Sun, 22 Mar 2009 19:00:05 +0000 Subject: [PATCH] libbb: make history saving/loading concurrent-safe * all history writers always append (not overwrite) history files * they reload history if they detect that file length has changed since last write * they trim history file only when it grows 4 times longer than MAXLINES * they do this atomically by creating new file and renaming it to old Unfortunately, this comes at a price: function old new delta load_history - 346 +346 read_line_input 3155 3358 +203 new_line_input_t 17 31 +14 ...irrelevant small jitter... ------------------------------------------------------------------------------ (add/remove: 1/0 grow/shrink: 5/5 up/down: 573/-13) Total: 560 bytes --- include/libbb.h | 10 +++- libbb/lineedit.c | 142 ++++++++++++++++++++++++++++++++++++----------- 2 files changed, 117 insertions(+), 35 deletions(-) diff --git a/include/libbb.h b/include/libbb.h index a2042fe5c..7bf9469cb 100644 --- a/include/libbb.h +++ b/include/libbb.h @@ -1187,9 +1187,9 @@ unsigned long long bb_makedev(unsigned int major, unsigned int minor) FAST_FUNC; #if ENABLE_FEATURE_EDITING /* It's NOT just ENABLEd or disabled. It's a number: */ #ifdef CONFIG_FEATURE_EDITING_HISTORY -#define MAX_HISTORY (CONFIG_FEATURE_EDITING_HISTORY + 0) +# define MAX_HISTORY (CONFIG_FEATURE_EDITING_HISTORY + 0) #else -#define MAX_HISTORY 0 +# define MAX_HISTORY 0 #endif typedef struct line_input_t { int flags; @@ -1197,7 +1197,11 @@ typedef struct line_input_t { #if MAX_HISTORY int cnt_history; int cur_history; - USE_FEATURE_EDITING_SAVEHISTORY(const char *hist_file;) +#if ENABLE_FEATURE_EDITING_SAVEHISTORY + unsigned cnt_history_in_file; + off_t last_history_end; + const char *hist_file; +#endif char *history[MAX_HISTORY + 1]; #endif } line_input_t; diff --git a/libbb/lineedit.c b/libbb/lineedit.c index 3953cc904..af1b62764 100644 --- a/libbb/lineedit.c +++ b/libbb/lineedit.c @@ -990,64 +990,141 @@ static int get_next_history(void) } #if ENABLE_FEATURE_EDITING_SAVEHISTORY +/* We try to ensure that concurrent additions to the history + * do not overwrite each other, and that additions to the history + * by one user are noticed by others. + * Otherwise shell users get unhappy. + * + * History file is trimmed lazily, when it grows several times longer + * than configured MAX_HISTORY lines. + */ + /* state->flags is already checked to be nonzero */ -static void load_history(const char *fromfile) +static void load_history(void) { + char *temp_h[MAX_HISTORY]; + char *line; FILE *fp; - int hi; + unsigned idx, i, line_len; /* NB: do not trash old history if file can't be opened */ - fp = fopen_for_read(fromfile); + fp = fopen_for_read(state->hist_file); if (fp) { /* clean up old history */ - for (hi = state->cnt_history; hi > 0;) { - hi--; - free(state->history[hi]); - state->history[hi] = NULL; + for (idx = state->cnt_history; idx > 0;) { + idx--; + free(state->history[idx]); + state->history[idx] = NULL; } - for (hi = 0; hi < MAX_HISTORY;) { - char *hl = xmalloc_fgetline(fp); - int l; - - if (!hl) - break; - l = strlen(hl); - if (l >= MAX_LINELEN) - hl[MAX_LINELEN-1] = '\0'; - if (l == 0) { - free(hl); + /* fill temp_h[], retaining only last MAX_HISTORY lines */ + memset(temp_h, 0, sizeof(temp_h)); + state->cnt_history_in_file = idx = 0; + while ((line = xmalloc_fgetline(fp)) != NULL) { + if (line[0] == '\0') { + free(line); continue; } - state->history[hi++] = hl; + free(temp_h[idx]); + temp_h[idx] = line; + state->cnt_history_in_file++; + idx++; + if (idx == MAX_HISTORY) + idx = 0; } + state->last_history_end = lseek(fileno(fp), 0, SEEK_CUR); fclose(fp); - state->cnt_history = hi; + + /* find first non-NULL temp_h[], if any */ + if (state->cnt_history_in_file) { + while (temp_h[idx] == NULL) { + idx++; + if (idx == MAX_HISTORY) + idx = 0; + } + } + + /* copy temp_h[] to state->history[] */ + for (i = 0; i < MAX_HISTORY;) { + line = temp_h[idx]; + if (!line) + break; + idx++; + if (idx == MAX_HISTORY) + idx = 0; + line_len = strlen(line); + if (line_len >= MAX_LINELEN) + line[MAX_LINELEN-1] = '\0'; + state->history[i++] = line; + } + state->cnt_history = i; } } /* state->flags is already checked to be nonzero */ -static void save_history(const char *tofile) +static void save_history(char *str) { - FILE *fp; + off_t end; + int fd; + int len; - fp = fopen_for_write(tofile); - if (fp) { - int i; + len = strlen(str); + again: + fd = open(state->hist_file, O_WRONLY | O_CREAT | O_APPEND, 0666); + if (fd < 0) + return; - for (i = 0; i < state->cnt_history; i++) { - fprintf(fp, "%s\n", state->history[i]); + end = lseek(fd, 0, SEEK_END); + + if (str) { + str[len] = '\n'; + full_write(fd, str, len + 1); + str[len] = '\0'; + str = NULL; + state->cnt_history_in_file++; + } + close(fd); + + /* if it was not a 1st write */ + if (state->last_history_end >= 0) { + /* did someone else write anything there? */ + if (state->last_history_end != end) { + load_history(); /* note: updates cnt_history_in_file */ + state->last_history_end = -1; + goto again; } - fclose(fp); + } + state->last_history_end = end + len + 1; + + /* did we write so much that history file needs trimming? */ + if (state->cnt_history_in_file > MAX_HISTORY * 4) { + FILE *fp; + char *new_name; + + new_name = xasprintf("%s.new", state->hist_file); + fp = fopen_for_write(new_name); + if (fp) { + int i; + + for (i = 0; i < state->cnt_history; i++) { + fprintf(fp, "%s\n", state->history[i]); + } + state->cnt_history_in_file = i; /* == cnt_history */ + state->last_history_end = lseek(fileno(fp), 0, SEEK_CUR); + fclose(fp); + /* replace hist_file atomically */ + rename(new_name, state->hist_file); + } + free(new_name); } } #else -#define load_history(a) ((void)0) +#define load_history() ((void)0) #define save_history(a) ((void)0) #endif /* FEATURE_COMMAND_SAVEHISTORY */ -static void remember_in_history(const char *str) +static void remember_in_history(char *str) { int i; @@ -1078,7 +1155,7 @@ static void remember_in_history(const char *str) state->cnt_history = i; #if ENABLE_FEATURE_EDITING_SAVEHISTORY if ((state->flags & SAVE_HISTORY) && state->hist_file) - save_history(state->hist_file); + save_history(str); #endif USE_FEATURE_EDITING_FANCY_PROMPT(num_ok_lines++;) } @@ -1413,7 +1490,7 @@ int FAST_FUNC read_line_input(const char *prompt, char *command, int maxsize, li state = st ? st : (line_input_t*) &const_int_0; #if ENABLE_FEATURE_EDITING_SAVEHISTORY if ((state->flags & SAVE_HISTORY) && state->hist_file) - load_history(state->hist_file); + load_history(); #endif if (state->flags & DO_HISTORY) state->cur_history = state->cnt_history; @@ -1875,6 +1952,7 @@ line_input_t* FAST_FUNC new_line_input_t(int flags) { line_input_t *n = xzalloc(sizeof(*n)); n->flags = flags; + n->last_history_end = -1; return n; }