Re: [hatari-devel] Recent Hatari change problems

[Eero Tamminen <oak@xxxxxxxxxxxxxx>]

Hi,

On 04/28/2018 01:07 AM, Thomas Huth wrote:
> Am Wed, 25 Apr 2018 01:17:59 +0300
> schrieb Eero Tamminen <oak@xxxxxxxxxxxxxx>:
> > I commited updated version of the change and some other
> > output reductions for the default log level.
> >
> > Few of these probably should also be using Log_Printf():
> > 	grep printf src/*.c|grep -i warn
>
> Yes, they need closer manual inspection, but we should likely convert
> them to Log_Printf, too.

Done, also to many other places.  Please mail if you notice
anything funky.

Everything relevant should now be converted to use logging
except for CPU core / floppy / hd image code.

Logging functions add now log level prefix to all log
messages so it's easy to seem what isn't using logging
functions.


> > Don't use find.  In my case your test wanted to wrangle with ~500
> > shell scripts (I have some of my own tests stuff in subdirectories
> > in my local Hatari Mercurial repo, and they include symlinks
> > elsewhere).
>
> I hope you found some more bashisms that way :-D

Err... Yes, a lot.  Just not in my own scripts, so I ignored it. :-)


> > As all the shell files are now fine, I think best would be
> > to have *one* test (script) that checks all shell scripts
>
> Yes, makes sense. I was also not very happy about having to
> encode the file name as test name, so I've put the stuff now
> into a separate script as you suggested.

It's a bit slow, compared to other tests.

Does CTest support multiple test targets, so that one could
split tests e.g. to sets that are run before release (all),
and things that should be run before commits?

IMHO checkbashism is more of a release check than commit check.


Alternatively, is there some nice way to run checks in parallel
from a shell script, while still collecting their error codes
and not exiting from the script until all checks have finished?


> > If you check shell scripts, you might also consider checking
> > relevant Python scripts for v2 & v3 compatibility:
....
> Sorry, I've got no clue about Python and pylint, so don't expect
> any test from my side.

Ok.  I'll look into it more at some later late.

(I hope they fix pylint first.)


	- Eero

diff -r db8954f039c2 src/debug/log.h
--- a/src/debug/log.h	Sun Apr 29 15:32:15 2018 +0300
+++ b/src/debug/log.h	Sun Apr 29 19:13:52 2018 +0300
@@ -63,6 +63,9 @@
 	LOG_NONE	/* invalid LOG level */
 } LOGTYPE;
 
+#define LOG_NAMES {"FATAL", "ERROR", "WARNING", "INFO", "TODO", NULL}
+
+
 #ifndef __GNUC__
 /* assuming attributes work only for GCC */
 #define __attribute__(foo)
diff -r db8954f039c2 src/debug/log.c
--- a/src/debug/log.c	Sun Apr 29 15:32:15 2018 +0300
+++ b/src/debug/log.c	Sun Apr 29 19:13:52 2018 +0300
@@ -20,6 +20,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <ctype.h>
+#include <assert.h>
 
 #include "main.h"
 #include "configuration.h"
@@ -229,6 +230,20 @@
 
 /*-----------------------------------------------------------------------*/
 /**
+ * Print log prefix when needed
+ */
+static void Log_PrintPrefix(FILE *fp, LOGTYPE idx)
+{
+	static const char* prefix[] = LOG_NAMES;
+
+	assert(idx >= 0 && idx < ARRAY_SIZE(prefix));
+	if (prefix[idx])
+		fprintf(fp, "%s: ", prefix[idx]);
+}
+
+
+/*-----------------------------------------------------------------------*/
+/**
  * Output string to log file
  */
 void Log_Printf(LOGTYPE nType, const char *psFormat, ...)
@@ -237,6 +252,7 @@
 
 	if (hLogFile && nType <= TextLogLevel)
 	{
+		Log_PrintPrefix(hLogFile, nType);
 		va_start(argptr, psFormat);
 		vfprintf(hLogFile, psFormat, argptr);
 		va_end(argptr);
@@ -258,6 +274,7 @@
 	/* Output to log file: */
 	if (hLogFile && nType <= TextLogLevel)
 	{
+		Log_PrintPrefix(hLogFile, nType);
 		va_start(argptr, psFormat);
 		vfprintf(hLogFile, psFormat, argptr);
 		va_end(argptr);