Re: marco's plaintext history patch
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=131490865525379=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 - 1.15
+++ bin/ksh/alloc.c 29 May 2017 09:47:43 -
@@ -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 - 1.58
+++ bin/ksh/history.c 29 May 2017 09:47:43 -
@@ -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
@@ -22,25 +21,16 @@
#include
#include
#include
+#include
#include "sh.h"
#ifdef HISTORY
-# include
-/*
- * 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 voidwritehistfile(void);
+static FILE*history_open(void);
+static int history_load(Source *);
+static voidhistory_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 voidhistbackup(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_tline_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)
+
Re: marco's plaintext history patch
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=131490865525379=2 > In history_load() is there a reason for casting line? + histsave(line_co, (char *)line, 0);
marco's plaintext history patch
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=131490865525379=2
Index: alloc.c
===
RCS file: /var/cvs/src/bin/ksh/alloc.c,v
retrieving revision 1.15
diff -u -p -r1.15 alloc.c
--- alloc.c 1 Jun 2016 10:29:20 - 1.15
+++ alloc.c 22 Feb 2017 21:19:37 -
@@ -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: history.c
===
RCS file: /var/cvs/src/bin/ksh/history.c,v
retrieving revision 1.58
diff -u -p -r1.58 history.c
--- history.c 24 Aug 2016 16:09:40 - 1.58
+++ history.c 22 Feb 2017 21:19:37 -
@@ -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
@@ -22,25 +21,16 @@
#include
#include
#include
+#include
#include "sh.h"
#ifdef HISTORY
-# include
-/*
- * 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 voidwritehistfile(void);
+static FILE*history_open(void);
+static int history_load(Source *);
+static voidhistory_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 voidhistbackup(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_tline_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
