On Fri, Apr 28, 2017 at 10:01:06AM +0200, Theo Buehler wrote:
> This is an updated version of an old patch by marco [1], including some
> improvements and fixes by natano and me. Here's marco's description:
>
> > I have had enough of corrupt ksh history so I had a look at the code to
> > try to fix it. The magical code was very magical so I basically deleted
> > most of it and made ksh history into a flat text file. It handles
> > multiple ksh instances writing to the same text file with locks just
> > like the current ksh does. I haven't noticed any differences in
> > behavior running this.
>
> If you wish to retain your current ksh history, you can create a
> plaintext version of it using a command like this (assuming HISTFILE and
> HISTSIZE are set):
>
> cp $HISTFILE $HISTFILE.old
> fc -ln 1 | sed 's/^ //' > ~/ksh_hist.txt
>
> Then apply the patch and install the new ksh.
>
> Once you've exited all interactive sessions with the old ksh (a reboot is
> probably your safest bet), you can use ksh_hist.txt as your history file.
>
> Note that the old ksh recklessly truncates a history file that it
> doesn't understand, so be careful not to run old and new interactive ksh
> processes in parallel.
>
> [1]: https://marc.info/?l=openbsd-tech&m=131490865525379&w=2
I had no feedback on this. Here's the patch again with the extra cast
(pointed out by Michael W. Bombardieri) removed.
Index: bin/ksh/alloc.c
===================================================================
RCS file: /cvs/src/bin/ksh/alloc.c,v
retrieving revision 1.15
diff -u -p -r1.15 alloc.c
--- bin/ksh/alloc.c 1 Jun 2016 10:29:20 -0000 1.15
+++ bin/ksh/alloc.c 29 May 2017 09:47:43 -0000
@@ -47,7 +47,7 @@ alloc(size_t size, Area *ap)
if (size > SIZE_MAX - sizeof(struct link))
internal_errorf(1, "unable to allocate memory");
- l = malloc(sizeof(struct link) + size);
+ l = calloc(1, sizeof(struct link) + size);
if (l == NULL)
internal_errorf(1, "unable to allocate memory");
l->next = ap->freelist;
Index: bin/ksh/history.c
===================================================================
RCS file: /cvs/src/bin/ksh/history.c,v
retrieving revision 1.58
diff -u -p -r1.58 history.c
--- bin/ksh/history.c 24 Aug 2016 16:09:40 -0000 1.58
+++ bin/ksh/history.c 29 May 2017 09:47:43 -0000
@@ -9,8 +9,7 @@
* a) the original in-memory history mechanism
* b) a more complicated mechanism done by p...@hillside.co.uk
* that more closely follows the real ksh way of doing
- * things. You need to have the mmap system call for this
- * to work on your system
+ * things.
*/
#include <sys/stat.h>
@@ -22,25 +21,16 @@
#include <stdio.h>
#include <string.h>
#include <unistd.h>
+#include <vis.h>
#include "sh.h"
#ifdef HISTORY
-# include <sys/mman.h>
-/*
- * variables for handling the data file
- */
-static int histfd;
-static int hsize;
-
-static int hist_count_lines(unsigned char *, int);
-static int hist_shrink(unsigned char *, int);
-static unsigned char *hist_skip_back(unsigned char *,int *,int);
-static void histload(Source *, unsigned char *, int);
-static void histinsert(Source *, int, unsigned char *);
-static void writehistfile(int, char *);
-static int sprinkle(int);
+static void writehistfile(void);
+static FILE *history_open(void);
+static int history_load(Source *);
+static void history_close(void);
static int hist_execute(char *);
static int hist_replace(char **, const char *, const char *, int);
@@ -48,11 +38,14 @@ static char **hist_get(const char *, i
static char **hist_get_oldest(void);
static void histbackup(void);
+static FILE *histfd;
static char **current; /* current position in history[] */
static char *hname; /* current name of history file */
static int hstarted; /* set after hist_init() called */
static Source *hist_source;
+static uint32_t line_co;
+static struct stat last_sb;
int
c_fc(char **wp)
@@ -546,15 +539,10 @@ sethistfile(const char *name)
/* if the name is the same as the name we have */
if (hname && strcmp(hname, name) == 0)
return;
-
/*
* its a new name - possibly
*/
- if (histfd) {
- /* yes the file is open */
- (void) close(histfd);
- histfd = 0;
- hsize = 0;
+ if (hname) {
afree(hname, APERM);
hname = NULL;
/* let's reset the history */
@@ -562,6 +550,7 @@ sethistfile(const char *name)
hist_source->line = 0;
}
+ history_close();
hist_init(hist_source);
}
@@ -578,6 +567,16 @@ init_histvec(void)
}
}
+static void
+history_lock(int operation)
+{
+ while (flock(fileno(histfd), operation) != 0) {
+ if (errno == EINTR || errno == EAGAIN)
+ continue;
+ else
+ break;
+ }
+}
/*
* Routines added by Peter Collinson BSDI(Europe)/Hillside Systems to
@@ -594,18 +593,28 @@ init_histvec(void)
void
histsave(int lno, const char *cmd, int dowrite)
{
- char **hp;
- char *c, *cp;
+ struct stat sb;
+ char **hp, *c, *cp, *encoded;
+
+ if (dowrite && histfd) {
+ history_lock(LOCK_EX);
+ if (fstat(fileno(histfd), &sb) != -1) {
+ if (timespeccmp(&sb.st_mtim, &last_sb.st_mtim, ==))
+ ; /* file is unchanged */
+ else {
+ /* reset history */
+ histptr = history - 1;
+ hist_source->line = 0;
+ history_load(hist_source);
+ }
+ }
+ }
c = str_save(cmd, APERM);
- if ((cp = strchr(c, '\n')) != NULL)
+ if ((cp = strrchr(c, '\n')) != NULL)
*cp = '\0';
- if (histfd && dowrite)
- writehistfile(lno, c);
-
hp = histptr;
-
if (++hp >= history + histsize) { /* remove oldest command */
afree(*history, APERM);
for (hp = history; hp < history + histsize - 1; hp++)
@@ -613,365 +622,148 @@ histsave(int lno, const char *cmd, int d
}
*hp = c;
histptr = hp;
-}
-/*
- * Write history data to a file nominated by HISTFILE. If HISTFILE
- * is unset then history is still recorded, but the data is not
- * written to a file. All copies of ksh looking at the file will
- * maintain the same history. This is ksh behaviour.
- */
-
-/*
- * History file format:
- * Bytes 1, 2: HMAGIC - just to check that we are dealing with
- the correct object
- * Each command, in the format:
- <command byte><command number(4 bytes)><bytes><null>
- */
-#define HMAGIC1 0xab
-#define HMAGIC2 0xcd
-#define COMMAND 0xff
+ if (dowrite && histfd) {
+ /* append to file */
+ if (fseeko(histfd, 0, SEEK_END) == 0 &&
+ stravis(&encoded, c, VIS_SAFE | VIS_NL) != -1) {
+ fprintf(histfd, "%s\n", encoded);
+ fflush(histfd);
+ fstat(fileno(histfd), &last_sb);
+ line_co++;
+ writehistfile();
+ free(encoded);
+ }
+ history_lock(LOCK_UN);
+ }
+}
-void
-hist_init(Source *s)
+static FILE *
+history_open(void)
{
- unsigned char *base;
- int lines;
- int fd;
- struct stat sb;
-
- if (Flag(FTALKING) == 0)
- return;
-
- hstarted = 1;
-
- hist_source = s;
+ int fd, fddup;
+ FILE *f;
+ struct stat sb;
- hname = str_val(global("HISTFILE"));
- if (hname == NULL)
- return;
- hname = str_save(hname, APERM);
-
- retry:
- /* we have a file and are interactive */
- if ((fd = open(hname, O_RDWR|O_CREAT|O_APPEND, 0600)) < 0)
- return;
+ if ((fd = open(hname, O_RDWR | O_CREAT | O_EXLOCK, 0600)) == -1)
+ return NULL;
if (fstat(fd, &sb) == -1 || sb.st_uid != getuid()) {
close(fd);
- return;
+ return NULL;
}
-
- histfd = savefd(fd);
- if (histfd != fd)
+ fddup = savefd(fd);
+ if (fddup != fd)
close(fd);
- (void) flock(histfd, LOCK_EX);
+ if ((f = fdopen(fddup, "r+")) == NULL)
+ close(fddup);
+ else
+ last_sb = sb;
- hsize = lseek(histfd, 0L, SEEK_END);
+ return f;
+}
- if (hsize == 0) {
- /* add magic */
- if (sprinkle(histfd)) {
- hist_finish();
- return;
- }
- }
- else if (hsize > 0) {
- /*
- * we have some data
- */
- base = mmap(0, hsize, PROT_READ,
- MAP_FILE|MAP_PRIVATE, histfd, 0);
- /*
- * check on its validity
- */
- if (base == MAP_FAILED || *base != HMAGIC1 || base[1] !=
HMAGIC2) {
- if (base != MAP_FAILED)
- munmap((caddr_t)base, hsize);
- hist_finish();
- if (unlink(hname) != 0)
- return;
- goto retry;
- }
- if (hsize > 2) {
- lines = hist_count_lines(base+2, hsize-2);
- if (lines > histsize) {
- /* we need to make the file smaller */
- if (hist_shrink(base, hsize))
- if (unlink(hname) != 0)
- return;
- munmap((caddr_t)base, hsize);
- hist_finish();
- goto retry;
- }
- }
- histload(hist_source, base+2, hsize-2);
- munmap((caddr_t)base, hsize);
+static void
+history_close(void)
+{
+ if (histfd) {
+ fflush(histfd);
+ fclose(histfd);
+ histfd = NULL;
}
- (void) flock(histfd, LOCK_UN);
- hsize = lseek(histfd, 0L, SEEK_END);
}
-typedef enum state {
- shdr, /* expecting a header */
- sline, /* looking for a null byte to end the line */
- sn1, /* bytes 1 to 4 of a line no */
- sn2, sn3, sn4
-} State;
-
static int
-hist_count_lines(unsigned char *base, int bytes)
+history_load(Source *s)
{
- State state = shdr;
- int lines = 0;
+ char *p, encoded[LINE + 1], line[LINE + 1];
+
+ rewind(histfd);
- while (bytes--) {
- switch (state) {
- case shdr:
- if (*base == COMMAND)
- state = sn1;
+ /* just read it all; will auto resize history upon next command */
+ for (line_co = 1; ; line_co++) {
+ p = fgets(encoded, sizeof(encoded), histfd);
+ if (p == NULL || feof(histfd) || ferror(histfd))
break;
- case sn1:
- state = sn2; break;
- case sn2:
- state = sn3; break;
- case sn3:
- state = sn4; break;
- case sn4:
- state = sline; break;
- case sline:
- if (*base == '\0')
- lines++, state = shdr;
+ if ((p = strchr(encoded, '\n')) == NULL) {
+ bi_errorf("history file is corrupt");
+ return 1;
}
- base++;
+ *p = '\0';
+ s->line = line_co;
+ s->cmd_offset = line_co;
+ strunvis(line, encoded);
+ histsave(line_co, line, 0);
}
- return lines;
-}
-/*
- * Shrink the history file to histsize lines
- */
-static int
-hist_shrink(unsigned char *oldbase, int oldbytes)
-{
- int fd;
- char nfile[1024];
- unsigned char *nbase = oldbase;
- int nbytes = oldbytes;
+ writehistfile();
- nbase = hist_skip_back(nbase, &nbytes, histsize);
- if (nbase == NULL)
- return 1;
- if (nbase == oldbase)
- return 0;
+ return 0;
+}
- /*
- * create temp file
- */
- (void) shf_snprintf(nfile, sizeof(nfile), "%s.%d", hname, procpid);
- if ((fd = open(nfile, O_CREAT | O_TRUNC | O_WRONLY, 0600)) < 0)
- return 1;
+void
+hist_init(Source *s)
+{
+ if (Flag(FTALKING) == 0)
+ return;
- if (sprinkle(fd)) {
- close(fd);
- unlink(nfile);
- return 1;
- }
- if (write(fd, nbase, nbytes) != nbytes) {
- close(fd);
- unlink(nfile);
- return 1;
- }
- close(fd);
+ hstarted = 1;
- /*
- * rename
- */
- if (rename(nfile, hname) < 0)
- return 1;
- return 0;
-}
+ hist_source = s;
+ hname = str_val(global("HISTFILE"));
+ if (hname == NULL)
+ return;
+ hname = str_save(hname, APERM);
+ histfd = history_open();
+ if (histfd == NULL)
+ return;
-/*
- * find a pointer to the data `no' back from the end of the file
- * return the pointer and the number of bytes left
- */
-static unsigned char *
-hist_skip_back(unsigned char *base, int *bytes, int no)
-{
- int lines = 0;
- unsigned char *ep;
+ history_load(s);
- for (ep = base + *bytes; --ep > base; ) {
- /* this doesn't really work: the 4 byte line number that is
- * encoded after the COMMAND byte can itself contain the
- * COMMAND byte....
- */
- for (; ep > base && *ep != COMMAND; ep--)
- ;
- if (ep == base)
- break;
- if (++lines == no) {
- *bytes = *bytes - ((char *)ep - (char *)base);
- return ep;
- }
- }
- return NULL;
+ history_lock(LOCK_UN);
}
-/*
- * load the history structure from the stored data
- */
static void
-histload(Source *s, unsigned char *base, int bytes)
+writehistfile(void)
{
- State state;
- int lno = 0;
- unsigned char *line = NULL;
-
- for (state = shdr; bytes-- > 0; base++) {
- switch (state) {
- case shdr:
- if (*base == COMMAND)
- state = sn1;
- break;
- case sn1:
- lno = (((*base)&0xff)<<24);
- state = sn2;
- break;
- case sn2:
- lno |= (((*base)&0xff)<<16);
- state = sn3;
- break;
- case sn3:
- lno |= (((*base)&0xff)<<8);
- state = sn4;
- break;
- case sn4:
- lno |= (*base)&0xff;
- line = base+1;
- state = sline;
- break;
- case sline:
- if (*base == '\0') {
- /* worry about line numbers */
- if (histptr >= history && lno-1 != s->line) {
- /* a replacement ? */
- histinsert(s, lno, line);
- }
- else {
- s->line = lno;
- s->cmd_offset = lno;
- histsave(lno, (char *)line, 0);
- }
- state = shdr;
- }
- }
- }
-}
+ char *cmd, *encoded;
+ int i;
-/*
- * Insert a line into the history at a specified number
- */
-static void
-histinsert(Source *s, int lno, unsigned char *line)
-{
- char **hp;
+ /* see if file has grown over 25% */
+ if (line_co < histsize + (histsize / 4))
+ return;
- if (lno >= s->line-(histptr-history) && lno <= s->line) {
- hp = &histptr[lno-s->line];
- afree(*hp, APERM);
- *hp = str_save((char *)line, APERM);
- }
-}
+ /* rewrite the whole caboodle */
+ rewind(histfd);
+ if (ftruncate(fileno(histfd), 0) == -1) {
+ bi_errorf("failed to rewrite history file - %s",
+ strerror(errno));
+ }
+ for (i = 0; i < histsize; i++) {
+ cmd = history[i];
+ if (cmd == NULL)
+ break;
-/*
- * write a command to the end of the history file
- * This *MAY* seem easy but it's also necessary to check
- * that the history file has not changed in size.
- * If it has - then some other shell has written to it
- * and we should read those commands to update our history
- */
-static void
-writehistfile(int lno, char *cmd)
-{
- int sizenow;
- unsigned char *base;
- unsigned char *new;
- int bytes;
- unsigned char hdr[5];
- struct iovec iov[2];
-
- (void) flock(histfd, LOCK_EX);
- sizenow = lseek(histfd, 0L, SEEK_END);
- if (sizenow != hsize) {
- /*
- * Things have changed
- */
- if (sizenow > hsize) {
- /* someone has added some lines */
- bytes = sizenow - hsize;
- base = mmap(0, sizenow,
- PROT_READ, MAP_FILE|MAP_PRIVATE, histfd, 0);
- if (base == MAP_FAILED)
- goto bad;
- new = base + hsize;
- if (*new != COMMAND) {
- munmap((caddr_t)base, sizenow);
- goto bad;
+ if (stravis(&encoded, cmd, VIS_SAFE | VIS_NL) != -1) {
+ if (fprintf(histfd, "%s\n", encoded) == -1) {
+ free(encoded);
+ return;
}
- hist_source->line--;
- histload(hist_source, new, bytes);
- hist_source->line++;
- lno = hist_source->line;
- munmap((caddr_t)base, sizenow);
- hsize = sizenow;
- } else {
- /* it has shrunk */
- /* but to what? */
- /* we'll give up for now */
- goto bad;
+ free(encoded);
}
}
- /*
- * we can write our bit now
- */
- hdr[0] = COMMAND;
- hdr[1] = (lno>>24)&0xff;
- hdr[2] = (lno>>16)&0xff;
- hdr[3] = (lno>>8)&0xff;
- hdr[4] = lno&0xff;
- iov[0].iov_base = hdr;
- iov[0].iov_len = 5;
- iov[1].iov_base = cmd;
- iov[1].iov_len = strlen(cmd) + 1;
- (void) writev(histfd, iov, 2);
- hsize = lseek(histfd, 0L, SEEK_END);
- (void) flock(histfd, LOCK_UN);
- return;
-bad:
- hist_finish();
+
+ line_co = histsize;
+
+ fflush(histfd);
+ fstat(fileno(histfd), &last_sb);
}
void
hist_finish(void)
{
- (void) flock(histfd, LOCK_UN);
- (void) close(histfd);
- histfd = 0;
-}
-
-/*
- * add magic to the history file
- */
-static int
-sprinkle(int fd)
-{
- static unsigned char mag[] = { HMAGIC1, HMAGIC2 };
-
- return(write(fd, mag, 2) != 2);
+ history_close();
}
#else /* HISTORY */
