Re: mg(1) log internals to file

2019-06-11 Thread Florian Obser
On Fri, Jun 07, 2019 at 03:18:27PM +, Mark Lumsden wrote:
> This diff allows mg to log its internal status to a file. At the moment it
> only logs line information like front and back pointers in the linked list,
> how many characters are used and where the cursor is placed in the file, but
> I am finding it incredibly useful debugging mg at the moment.
> 
> Below is an example log file after opening a file with this contents:
> 
> abc
> def
> ghk
> 
> After I open the file in mg, I move forward a line (C-n), then move forward
> a character (C-f):
> 
> $ cat log/mg.log
> next-line
> > 3 3 0xc3a3557ae00 b^0xc3a592eac20 f.0xc3a592ea4a0 abc
>  3 3 0xc3a592ea4a0 b^0xc3a3557ae00 f.0xc3a592ea660 def
>  3 3 0xc3a592ea660 b^0xc3a592ea4a0 f.0xc3a307830e0 ghk
>  0 0 0xc3a307830e0 b^0xc3a592ea660 f.0xc3a592eac20 (null)
>  head 0xc3a592eac20 b^0xc3a307830e0 f.0xc3a3557ae00
> (EOB)
> 
> forward-char
>  3 3 0xc3a3557ae00 b^0xc3a592eac20 f.0xc3a592ea4a0 abc
> > 3 3 0xc3a592ea4a0 b^0xc3a3557ae00 f.0xc3a592ea660 def
>  3 3 0xc3a592ea660 b^0xc3a592ea4a0 f.0xc3a307830e0 ghk
>  0 0 0xc3a307830e0 b^0xc3a592ea660 f.0xc3a592eac20 (null)
>  head 0xc3a592eac20 b^0xc3a307830e0 f.0xc3a3557ae00
> (EOB)
> 
> The three columns of pointer addresses are (from the 'def' line):
> 
> 0xc3a592ea4a0 the lines own pointer.
> 0xc3a3557ae00 the pointer to back(b^) line
> 0xc3a592ea660 the pointer to forward(f.) line
> 
> The numbers at the start of the first four lines are l_size and l_used.
> 
> With this diff logging is not switched on by default and has to compiled
> into mg.  -DMGDEBUG has to be added to the Makefile:
> 
> CFLAGS+=-Wall -DREGEX  -DMGDEBUG
> 
> And the comment removed from:
> 
> #SRCS+=  log.c
> 
> The information logged at the moment is viewable by gdb, however, if a bug
> that I have introduced doesn't present itself for XX number of key presses,
> I can struggle to work out what has caused it in gdb, logging like this
> makes problems I have introduced much more obvious and easier to fix.
> 
> I have tried to make the ifdefs as minimal as possible. Any objections to
> this diff?
> 

I'm not fundamentally opposed to such a functionality since I have
written an ad-hoc version of this on multiple occasions.

That being said, I would prefer a version that is so non-invasive that
we can compile it in always so that it does not fall to bitrot. I'd
also like an api that allows me to log arbitrary strings in addition
to what you have written here.

It does not quite live up to OpenBSD coding standards, see inline
(ignoring that your mail client seemed to have mangled the diff).

> Mark
> 
> Index: Makefile
> ===
> RCS file: /cvs/src/usr.bin/mg/Makefile,v
> retrieving revision 1.33
> diff -u -p -u -p -r1.33 Makefile
> --- Makefile  16 Sep 2016 17:17:40 -  1.33
> +++ Makefile  7 Jun 2019 14:42:39 -
> @@ -9,6 +9,7 @@ DPADD+=   ${LIBCURSES} ${LIBUTIL}
>  #
>  #REGEX   -- create regular expression functions.
>  #STARTUPFILE -- look for and handle initialization file.
> +#MGDEBUG -- debug mg internals to a log file.
>  #
>  CFLAGS+=-Wall -DREGEX
> 
> @@ -22,6 +23,11 @@ SRCS=  autoexec.c basic.c bell.c buffer.c
>  # More or less standalone extensions.
>  #
>  SRCS+=   cmode.c cscope.c dired.c grep.c tags.c
> +
> +#
> +# -DMGDEBUG source file.
> +#
> +#SRCS+=  log.c
> 
>  afterinstall:
>   ${INSTALL} -d -o root -g wheel ${DESTDIR}${DOCDIR}/mg
> Index: kbd.c
> ===
> RCS file: /cvs/src/usr.bin/mg/kbd.c,v
> retrieving revision 1.30
> diff -u -p -u -p -r1.30 kbd.c
> --- kbd.c 26 Sep 2015 21:51:58 -  1.30
> +++ kbd.c 7 Jun 2019 14:42:39 -
> @@ -15,6 +15,10 @@
>  #include "key.h"
>  #include "macro.h"
> 
> +#ifdef  MGDEBUG
> +#include "log.h"
> +#endif
> +
>  #define METABIT 0x80
> 
>  #define PROMPTL 80
> @@ -151,6 +155,11 @@ doin(void)
>   while ((funct = doscan(curmap, (key.k_chars[key.k_count++] =
>   getkey(TRUE)), &curmap)) == NULL)
>   /* nothing */;
> +
> +#ifdef  MGDEBUG
> + if (!mglog(funct))
> + ewprintf("Problem with logging");
> +#endif
> 
>   if (macrodef && macrocount < MAXMACRO)
>   macro[macrocount++].m_funct = funct;
> Index: log.c
> ===
> RCS file: log.c
> diff -N log.c
> --- /dev/null 1 Jan 1970 00:00:00 -
> +++ log.c 7 Jun 2019 14:42:39 -
> @@ -0,0 +1,121 @@
> +/*   $OpenBSD$   */
> +
> +/* + * This file is in the public domain.
> + *
> + * Author: Mark Lumsden 
> + *
> + */
> +
> +/*
> + * Record a history of an mg session for temporal debugging.
> + * Sometimes pressing a key will set the scene for a bug only visible + *
> dozens of keystrokes later. gdb has its limitations in this scenario.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#includ

mg(1) log internals to file

2019-06-07 Thread Mark Lumsden
This diff allows mg to log its internal status to a file. At the moment it 
only logs line information like front and back pointers in the linked 
list, how many characters are used and where the cursor is placed in the 
file, but I am finding it incredibly useful debugging mg at the moment.


Below is an example log file after opening a file with this contents:

abc
def
ghk

After I open the file in mg, I move forward a line (C-n), then move 
forward a character (C-f):


$ cat log/mg.log
next-line

3 3 0xc3a3557ae00 b^0xc3a592eac20 f.0xc3a592ea4a0 abc

 3 3 0xc3a592ea4a0 b^0xc3a3557ae00 f.0xc3a592ea660 def
 3 3 0xc3a592ea660 b^0xc3a592ea4a0 f.0xc3a307830e0 ghk
 0 0 0xc3a307830e0 b^0xc3a592ea660 f.0xc3a592eac20 (null)
 head 0xc3a592eac20 b^0xc3a307830e0 f.0xc3a3557ae00
(EOB)

forward-char
 3 3 0xc3a3557ae00 b^0xc3a592eac20 f.0xc3a592ea4a0 abc

3 3 0xc3a592ea4a0 b^0xc3a3557ae00 f.0xc3a592ea660 def

 3 3 0xc3a592ea660 b^0xc3a592ea4a0 f.0xc3a307830e0 ghk
 0 0 0xc3a307830e0 b^0xc3a592ea660 f.0xc3a592eac20 (null)
 head 0xc3a592eac20 b^0xc3a307830e0 f.0xc3a3557ae00
(EOB)

The three columns of pointer addresses are (from the 'def' line):

0xc3a592ea4a0 the lines own pointer.
0xc3a3557ae00 the pointer to back(b^) line
0xc3a592ea660 the pointer to forward(f.) line

The numbers at the start of the first four lines are l_size and l_used.

With this diff logging is not switched on by default and has to compiled 
into mg.  -DMGDEBUG has to be added to the Makefile:


CFLAGS+=-Wall -DREGEX  -DMGDEBUG

And the comment removed from:

#SRCS+=  log.c

The information logged at the moment is viewable by gdb, however, if a bug 
that I have introduced doesn't present itself for XX number of key 
presses, I can struggle to work out what has caused it in gdb, logging 
like this makes problems I have introduced much more obvious and easier 
to fix.


I have tried to make the ifdefs as minimal as possible. Any objections to 
this diff?


Mark

Index: Makefile
===
RCS file: /cvs/src/usr.bin/mg/Makefile,v
retrieving revision 1.33
diff -u -p -u -p -r1.33 Makefile
--- Makefile16 Sep 2016 17:17:40 -  1.33
+++ Makefile7 Jun 2019 14:42:39 -
@@ -9,6 +9,7 @@ DPADD+= ${LIBCURSES} ${LIBUTIL}
 #
 #  REGEX   -- create regular expression functions.
 #  STARTUPFILE -- look for and handle initialization file.
+#  MGDEBUG -- debug mg internals to a log file.
 #
 CFLAGS+=-Wall -DREGEX

@@ -22,6 +23,11 @@ SRCS=autoexec.c basic.c bell.c buffer.c
 # More or less standalone extensions.
 #
 SRCS+= cmode.c cscope.c dired.c grep.c tags.c
+
+#
+# -DMGDEBUG source file.
+#
+#SRCS+=log.c

 afterinstall:
${INSTALL} -d -o root -g wheel ${DESTDIR}${DOCDIR}/mg
Index: kbd.c
===
RCS file: /cvs/src/usr.bin/mg/kbd.c,v
retrieving revision 1.30
diff -u -p -u -p -r1.30 kbd.c
--- kbd.c   26 Sep 2015 21:51:58 -  1.30
+++ kbd.c   7 Jun 2019 14:42:39 -
@@ -15,6 +15,10 @@
 #include "key.h"
 #include "macro.h"

+#ifdef  MGDEBUG
+#include "log.h"
+#endif
+
 #define METABIT 0x80

 #define PROMPTL 80
@@ -151,6 +155,11 @@ doin(void)
while ((funct = doscan(curmap, (key.k_chars[key.k_count++] =
getkey(TRUE)), &curmap)) == NULL)
/* nothing */;
+
+#ifdef  MGDEBUG
+   if (!mglog(funct))
+   ewprintf("Problem with logging");
+#endif

if (macrodef && macrocount < MAXMACRO)
macro[macrocount++].m_funct = funct;
Index: log.c
===
RCS file: log.c
diff -N log.c
--- /dev/null   1 Jan 1970 00:00:00 -
+++ log.c   7 Jun 2019 14:42:39 -
@@ -0,0 +1,121 @@
+/* $OpenBSD$   */
+
+/* 
+ * This file is in the public domain.

+ *
+ * Author: Mark Lumsden 
+ *
+ */
+
+/*
+ * Record a history of an mg session for temporal debugging.
+ * Sometimes pressing a key will set the scene for a bug only visible 
+ * dozens of keystrokes later. gdb has its limitations in this scenario.

+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "def.h"
+#include "log.h"
+#include "funmap.h"
+
+static charmglogpath[20];
+
+int
+mglog(PF funct)
+{
+   struct line *lp;
+   struct stat  sb;
+   char*curline;
+   FILE*fd;
+   int  i;
+
+   i = 0;
+
+   if(stat(mglogpath, &sb))
+   return (FALSE);
+
+   fd = fopen(mglogpath, "a");
+   if (fprintf(fd, "%s\n", function_name(funct)) == -1) {
+   fclose(fd);
+   return (FALSE);
+   }
+   lp = bfirstlp(curbp);
+
+   for(;;) {
+   i++;
+   curline = " ";
+   if (i == curwp->w_dotline)
+   curline = ">";
+   if (fprintf(fd, "%s%d %d %p b^%p f.%p %