Re: [Freeciv-Dev] patch: provide snprintf

[David Pfitzner <dwp@xxxxxxxxxxxxxx>]

Here is a revised version of a patch originally submitted by 
Markus Linnala <maage@xxxxxxxxx>, with some ideas I discussed 
previously.

I've added a new logging level LOG_VERBOSE, between LOG_NORMAL and
LOG_DEBUG.  The idea is that LOG_DEBUG is only if you have DEBUG
defined (eg, configure --enable-debug, or via CFLAGS), and VERBOSE
is more than "normal", but not everything.
(Currently enabled LOG_DEBUG messages should probably mostly be
changed to LOG_VERBOSE, and things currently if(0) LOG_DEBUG
should be changed to just LOG_DEBUG.  This patch does not yet do
that.  (Though note it was pointed out that some of the if(0) parts
may cause core dumps if enabled, due to using punit after punit 
freed etc.))

I moved the parsing of the --debug argument into the log module,
so it can be used by clients as well.

With this patch gcc generates a warning if you include log.h but
don't use freelog(); this revealed some unnecessary include lines
which I removed.

This patch also requires doc updates (README), not included.

Regards,
-- David

diff -u -r --exclude-from exclude freeciv-cvs/client/gui-gtk/chatline.c 
fc-adv/client/gui-gtk/chatline.c
--- freeciv-cvs/client/gui-gtk/chatline.c       Mon Apr 26 17:16:16 1999
+++ fc-adv/client/gui-gtk/chatline.c    Sun May 30 14:25:16 1999
@@ -15,7 +15,6 @@
 #include <string.h>
 
 #include <chatline.h>
-#include <log.h>
 #include <packets.h>
 #include <clinet.h>
 #include <gui_stuff.h>
diff -u -r --exclude-from exclude freeciv-cvs/client/gui-gtk/cityrep.c 
fc-adv/client/gui-gtk/cityrep.c
--- freeciv-cvs/client/gui-gtk/cityrep.c        Wed May 26 20:39:18 1999
+++ fc-adv/client/gui-gtk/cityrep.c     Sun May 30 14:25:31 1999
@@ -30,7 +30,6 @@
 #include <mapview.h>
 #include <citydlg.h>
 #include <optiondlg.h>
-#include <log.h>
 #include <cityrep.h>
 #include <options.h>
 
diff -u -r --exclude-from exclude freeciv-cvs/client/gui-gtk/gamedlgs.c 
fc-adv/client/gui-gtk/gamedlgs.c
--- freeciv-cvs/client/gui-gtk/gamedlgs.c       Mon May  3 22:39:27 1999
+++ fc-adv/client/gui-gtk/gamedlgs.c    Sun May 30 14:25:45 1999
@@ -31,7 +31,6 @@
 #include <dialogs.h>
 #include <chatline.h>
 #include <cityrep.h>
-#include <log.h>
 #include <options.h>
 
 extern GtkWidget *toplevel;
diff -u -r --exclude-from exclude freeciv-cvs/client/gui-gtk/gui_main.c 
fc-adv/client/gui-gtk/gui_main.c
--- freeciv-cvs/client/gui-gtk/gui_main.c       Sat May 29 11:31:55 1999
+++ fc-adv/client/gui-gtk/gui_main.c    Sun May 30 14:03:09 1999
@@ -171,7 +171,8 @@
     if (password)
       strcpy(name, password->pw_name);
     else {
-      freelog(LOG_NORMAL, "Your getpwuid call failed.  Please report this.");
+      /* freelog not yet initialised here --dwp */
+      fprintf(stderr, "Your getpwuid call failed.  Please report this.");
       strcpy(name, "operator 00000");
       sprintf(name+9, "%05i",(int)getuid());
    }
@@ -295,7 +296,10 @@
        }
         (*argv)[i] = NULL;
 
-       loglevel=atoi(opt_debug);
+       loglevel=log_parse_level_str(opt_debug);
+       if (loglevel==-1) {
+         exit(1);
+       }
       }
       else if (!strcmp ("--tiles",  (*argv)[i]) ||
               !strncmp ("--tiles=", (*argv)[i], 8) ||
diff -u -r --exclude-from exclude freeciv-cvs/client/gui-gtk/gui_stuff.c 
fc-adv/client/gui-gtk/gui_stuff.c
--- freeciv-cvs/client/gui-gtk/gui_stuff.c      Sat May 29 11:31:55 1999
+++ fc-adv/client/gui-gtk/gui_stuff.c   Sun May 30 14:26:02 1999
@@ -17,8 +17,6 @@
 #include <gtk/gtk.h>
 #include <gdk/gdkkeysyms.h>
 
-#include <log.h>
-
 extern GdkWindow *root_window;
 
 extern GdkGC *mask_fg_gc;
diff -u -r --exclude-from exclude freeciv-cvs/client/gui-gtk/mapview.c 
fc-adv/client/gui-gtk/mapview.c
--- freeciv-cvs/client/gui-gtk/mapview.c        Wed May 26 20:39:20 1999
+++ fc-adv/client/gui-gtk/mapview.c     Sun May 30 14:26:12 1999
@@ -23,7 +23,6 @@
 #include <game.h>
 #include <civclient.h>
 #include <graphics.h>
-#include <log.h>
 #include <mapview.h>
 #include <colors.h>
 #include <mapctrl.h>
diff -u -r --exclude-from exclude freeciv-cvs/client/gui-xaw/chatline.c 
fc-adv/client/gui-xaw/chatline.c
--- freeciv-cvs/client/gui-xaw/chatline.c       Mon Apr 26 17:16:16 1999
+++ fc-adv/client/gui-xaw/chatline.c    Sun May 30 12:28:35 1999
@@ -19,7 +19,6 @@
 #include <X11/Xaw/AsciiText.h>
 
 #include <chatline.h>
-#include <log.h>
 #include <packets.h>
 #include <clinet.h>
 #include <gui_stuff.h>
diff -u -r --exclude-from exclude freeciv-cvs/client/gui-xaw/gui_main.c 
fc-adv/client/gui-xaw/gui_main.c
--- freeciv-cvs/client/gui-xaw/gui_main.c       Wed May 26 22:03:01 1999
+++ fc-adv/client/gui-xaw/gui_main.c    Sun May 30 14:13:39 1999
@@ -85,8 +85,8 @@
       XtOffset(AppResources *,port), XtRImmediate, (XtPointer)False},
     { "server", "Server", XtRString, sizeof(String),
       XtOffset(AppResources *,server), XtRImmediate, (XtPointer)False},
-    { "logLevel", "LogLevel", XtRInt, sizeof(int),
-      XtOffset(AppResources *,loglevel), XtRImmediate, (XtPointer)False},
+    { "logLevel", "LogLevel", XtRString, sizeof(String),
+      XtOffset(AppResources *,loglevel_str), XtRImmediate, (XtPointer)False},
     { "version", "Version", XtRString, sizeof(String),
       XtOffset(AppResources *,version), XtRImmediate, (XtPointer)False},
 /*
@@ -264,7 +264,7 @@
 **************************************************************************/
 void ui_main(int argc, char *argv[])
 {
-  int i;
+  int i, loglevel;
   Pixmap icon_pixmap; 
   XtTranslations TextFieldTranslations;
   Dimension w,h;
@@ -284,8 +284,12 @@
 
   XtGetApplicationResources(toplevel, &appResources, resources,
                             XtNumber(resources), NULL, 0);
-  
-  log_init(appResources.logfile, appResources.loglevel, NULL);
+
+  loglevel = log_parse_level_str(appResources.loglevel_str);
+  if (loglevel==-1) {
+    exit(1);
+  }
+  log_init(appResources.logfile, loglevel, NULL);
 
 /*  XSynchronize(display, 1); 
   XSetErrorHandler(myerr);*/
diff -u -r --exclude-from exclude freeciv-cvs/client/gui-xaw/mapview.c 
fc-adv/client/gui-xaw/mapview.c
--- freeciv-cvs/client/gui-xaw/mapview.c        Thu May 20 22:33:41 1999
+++ fc-adv/client/gui-xaw/mapview.c     Sun May 30 12:28:18 1999
@@ -23,7 +23,6 @@
 #include <mapview.h>
 #include <map.h>
 #include <graphics.h>
-#include <log.h>
 #include <colors.h>
 #include <unit.h>
 #include <game.h>
diff -u -r --exclude-from exclude freeciv-cvs/client/gui-xaw/optiondlg.c 
fc-adv/client/gui-xaw/optiondlg.c
--- freeciv-cvs/client/gui-xaw/optiondlg.c      Mon May  3 22:39:28 1999
+++ fc-adv/client/gui-xaw/optiondlg.c   Sun May 30 12:28:48 1999
@@ -38,7 +38,6 @@
 #include <events.h>
 #include <chatline.h>
 #include <cityrep.h>
-#include <log.h>
 #include <options.h>
 
 extern Widget toplevel, main_form;
diff -u -r --exclude-from exclude freeciv-cvs/client/gui-xaw/resources.h 
fc-adv/client/gui-xaw/resources.h
--- freeciv-cvs/client/gui-xaw/resources.h      Tue Apr 13 20:53:13 1999
+++ fc-adv/client/gui-xaw/resources.h   Sun May 30 14:13:00 1999
@@ -21,7 +21,7 @@
   Boolean showHelp;
   Boolean showVersion;
   String  logfile;
-  int     loglevel;
+  String  loglevel_str;
   String  name;
   int     port;
   String  server;
diff -u -r --exclude-from exclude freeciv-cvs/common/log.c fc-adv/common/log.c
--- freeciv-cvs/common/log.c    Tue Apr 27 22:13:07 1999
+++ fc-adv/common/log.c Sun May 30 15:57:05 1999
@@ -14,11 +14,116 @@
 #include <stdarg.h>
 #include <string.h>
 
-#include <log.h>
+#include "log.h"
+#include "mem.h"
 
 int log_level;
 char *log_filename;
 log_callback_fn log_callback;
+int logd_init_counter = 1;
+
+struct logd_fileinfo {
+  char *name;
+  int min;
+  int max;
+};
+static int logd_num_files;
+static struct logd_fileinfo *logd_files;
+
+/**************************************************************************
+level_str should be either "0", "1", "2", "3" or
+"3:filename" or "3:file1:file2" or "3:filename,100,200" etc
+
+If everything goes ok, returns the level.
+If there was a parsing problem, prints to stderr, and returns -1.
+
+Also sets up the logd_files data structure and increments
+logd_init_counter.  Does _not_ set log_level.
+**************************************************************************/
+int log_parse_level_str(char *level_str)
+{
+  char *c, *dup, *tok;
+  int n = 0;                   /* number of filenames */
+  int i;
+  int level;
+
+  /* re-entrant: */
+  logd_num_files = 0;
+  if (logd_files) {
+    free(logd_files);
+    logd_files = NULL;
+  }
+  logd_init_counter++;
+  
+  c = level_str;
+  n = 0;
+  while((c = strchr(c, ':')) != NULL) {
+    c++;
+    n++;
+  }
+  if (n == 0) {
+    level = atoi(level_str);
+    if (level >= LOG_NORMAL && level <= LOG_DEBUG) {
+      return level;
+    } else {
+      fprintf(stderr, "Bad log level %d in \"%s\".\n", level, level_str);
+      return -1;
+    }
+  }
+
+  c = level_str;
+  if (c[0] == '3' && c[1] == ':') {
+    level = 3;
+  } else {
+    fprintf(stderr, "Badly formed log level argument \"%s\".\n", level_str);
+    return -1;
+  }
+  logd_num_files = n;
+  logd_files = fc_malloc(n * sizeof(struct logd_fileinfo));
+  
+  dup = mystrdup(c+2);
+  tok = strtok(dup, ":");
+  if (tok==NULL) {
+    fprintf(stderr, "Badly formed log level argument \"%s\".\n", level_str);
+    level = -1;
+    goto cleanup;
+  }
+  i = 0;
+  do {
+    logd_files[i].min = logd_files[i].max = 0;
+    c = strchr(tok, ',');
+    if (c != NULL) {
+      char *pc;
+      c[0] = '\0';
+      c++;
+      pc = c;
+      c = strchr(c, ',');
+      if (c != NULL && *pc && *(c+1)) {
+       c[0] = '\0';
+       logd_files[i].min = atoi(pc);
+       logd_files[i].max = atoi(c+1);
+      }
+    }
+    if(strlen(tok)==0) {
+      fprintf(stderr, "Empty filename in log level argument \"%s\".\n",
+             level_str);
+      level = -1;
+      goto cleanup;
+    }
+    logd_files[i].name = strdup(tok);
+    i++;
+    tok = strtok(NULL, ":");
+  } while(tok != NULL);
+
+  if (i!=logd_num_files) {
+    fprintf(stderr, "Badly formed log level argument \"%s\".\n", level_str);
+    level = -1;
+  }
+
+cleanup:
+  free(dup);
+  return level;
+}
 
 /**************************************************************************
 Initialise the log module.
@@ -31,7 +136,8 @@
   log_level=initial_level;
   log_filename=filename;
   log_callback=callback;
-  /* freelog(LOG_NORMAL, "log started"); */
+  freelog(LOG_VERBOSE, "log started");
+  freelog(LOG_DEBUG, "LOG_DEBUG test");
 }
 
 /**************************************************************************
@@ -43,6 +149,32 @@
 }
 
 /**************************************************************************
+  Return an updated struct logdebug_afile_info: 
+**************************************************************************/
+struct logdebug_afile_info logdebug_update(const char *file)
+{
+  struct logdebug_afile_info ret;
+  int i;
+
+  ret.this = 1;
+  ret.min = 0;
+  ret.max = 0;
+  if (logd_num_files==0) {
+    return ret;
+  }
+  ret.this = 0;
+  for (i = 0; i < logd_num_files; i++) {
+    if((strstr(file, logd_files[i].name) != NULL)) {
+      ret.this = 1;
+      ret.min = logd_files[i].min;
+      ret.max = logd_files[i].max;
+      return ret;
+    }
+  }
+  return ret;
+}
+
+/**************************************************************************
 Unconditionally print a simple string.
 Let the callback do its own level formating and add a '\n' if it wants.
 **************************************************************************/
@@ -63,7 +195,7 @@
 "last message repeated ..." at some later time.
 Calls log_callback if non-null, else prints to stderr.
 **************************************************************************/
-int freelog(int level, char *message, ...)
+void vreal_freelog(int level, char *message, va_list ap)
 {
   static char bufbuf[2][512];
   char buf[512];
@@ -74,7 +206,6 @@
   static int prev_level=-1;    /* only count as repeat if same level  */
 
   if(level<=log_level) {
-    va_list args;
     FILE *fs;
 
     if(log_filename) {
@@ -86,9 +217,7 @@
     }
     else fs=stderr;
 
-    va_start(args, message);
-    vsprintf(bufbuf[whichbuf], message, args);
-    va_end(args);
+    vsprintf(bufbuf[whichbuf], message, ap);
     
     if(level==prev_level && 0==strncmp(bufbuf[0],bufbuf[1],511)){
       repeated++;
@@ -129,6 +258,11 @@
     if(log_filename)
       fclose(fs);
   }
-
-  return 1;
+}
+void real_freelog(int level, char *message, ...)
+{
+  va_list ap;
+  va_start(ap, message);
+  vreal_freelog(level, message, ap);
+  va_end(ap);
 }
diff -u -r --exclude-from exclude freeciv-cvs/common/log.h fc-adv/common/log.h
--- freeciv-cvs/common/log.h    Tue Apr 27 22:13:07 1999
+++ fc-adv/common/log.h Sun May 30 14:04:15 1999
@@ -13,11 +13,35 @@
 #ifndef FC__LOG_H
 #define FC__LOG_H
 
+#include <stdarg.h>
+
 #include "attribute.h"
 
-#define LOG_FATAL  0
-#define LOG_NORMAL 1
-#define LOG_DEBUG  2
+#define LOG_FATAL   0
+#define LOG_NORMAL  1
+#define LOG_VERBOSE 2          /* not shown by default */
+#define LOG_DEBUG   3          /* suppressed unless DEBUG defined;
+                                  may be enabled on file/line basis */
+
+/* Some variables local to each file which includes log.h,
+   to record whether LOG_DEBUG messages apply for that file
+   and if so for which lines (min,max) :
+*/
+struct logdebug_afile_info {
+  int this;
+  int min;
+  int max;
+};
+#ifdef DEBUG
+static int logdebug_this_init;
+static struct logdebug_afile_info logdebug_thisfile;
+#endif
+
+extern int logd_init_counter;   /* increment this to force re-init */
+
+/* Return an updated struct logdebug_afile_info: */
+struct logdebug_afile_info logdebug_update(const char *file);
+
 
 /* A function type to enable custom output of log messages other than
  * via fputs(stderr).  Eg, to the server console while handling prompts,
@@ -25,10 +49,74 @@
  */
 typedef void (*log_callback_fn)(int, char*);
 
+int log_parse_level_str(char *level_str);
 void log_init(char *filename, int initial_level, log_callback_fn callback);
 void log_set_level(int level);
 
-int freelog(int level, char *message, ...)
-            fc__attribute((format (printf, 2, 3)));
+void real_freelog(int level, char *message, ...)
+                  fc__attribute((format (printf, 2, 3)));
+void vreal_freelog(int level, char *message, va_list ap);
+
+
+/* A static (per-file) function to use/update the above per-file vars.
+ * This should only be called for LOG_DEBUG messages.
+ * It returns whether such a LOG_DEBUG message should be sent on
+ * to real_freelog.
+ */
+#ifdef DEBUG
+static int logdebug_check(const char *file, int line)
+{
+  if (logdebug_this_init < logd_init_counter) {  
+    logdebug_thisfile = logdebug_update(file);
+    logdebug_this_init = logd_init_counter;
+  } 
+  return (logdebug_thisfile.this  && (logdebug_thisfile.max==0 
+                                     || (line >= logdebug_thisfile.min 
+                                         && line <= logdebug_thisfile.max))); 
+}
+/* Including log.h without calling freelog() can generate a
+   warning that logdebug_check is never used; can use this to
+   suppress that warning:
+*/
+#define logdebug_suppress_warning logdebug_check(__FILE__, __LINE__)
+#else
+#define logdebug_suppress_warning
+#endif
+
+/* For GCC we use a variadic macro; for others we take
+   the performance hit of a function to get variadic args:
+*/
+#ifdef __GNUC__
+#ifdef DEBUG
+#define freelog(level, args...) do { \
+  if ((level) != LOG_DEBUG || logdebug_check(__FILE__, __LINE__)) { \
+    real_freelog((level), ##args); \
+  } \
+} while(0) 
+#else
+#define freelog(level, args...) do { \
+  if ((level) != LOG_DEBUG) { \
+    real_freelog((level), ##args); } \
+} while(0) 
+#endif  /* DEBUG */
+#else
+/* non-GCC: */
+static void freelog(int level, char *message, ...)
+{
+  int log_this; 
+#ifdef DEBUG
+  log_this = (level != LOG_DEBUG || logdebug_check(__FILE__, __LINE__));
+#else
+  log_this = (level != LOG_DEBUG);
+#endif
+  if (log_this) {
+    va_list args;
+    va_start(args, message);
+    vreal_freelog(level, message, args);
+    va_end(args);
+  }
+}
+#endif /* __GNUC__ */
 
+           
 #endif
diff -u -r --exclude-from exclude freeciv-cvs/server/civserver.c 
fc-adv/server/civserver.c
--- freeciv-cvs/server/civserver.c      Mon May 24 19:12:05 1999
+++ fc-adv/server/civserver.c   Sun May 30 14:00:24 1999
@@ -128,7 +128,7 @@
   char *script_filename=NULL;
   int i;
   int save_counter=0;
-  int log_level=LOG_NORMAL;
+  int loglevel=LOG_NORMAL;
 
   if (!getuid() || !geteuid()) {
     fprintf(stderr, "%s: Fatal error: you're trying to run me as superuser!\n",
@@ -211,9 +211,14 @@
       }
     }
     else if(!strcmp("-d", argv[i]) || !strcmp("--debug", argv[i])) { 
-      if(++i<argc) 
-       log_level=atoi(argv[i]);
-      else {
+      if(++i<argc) {
+       loglevel = log_parse_level_str(argv[i]);
+       if (loglevel == -1) {
+         loglevel = LOG_NORMAL;
+         h=1;
+         break;
+       }
+      } else {
        fprintf(stderr, "Error: no debug log level specified.\n");
        h=1;
        break;
@@ -247,13 +252,13 @@
     fprintf(stderr, "  -p, --port N\t\t\tconnect to port N\n");
     fprintf(stderr, "  -r, --read\t\t\tRead startup script\n");
     fprintf(stderr, "  -s, --server H\t\tList this server as host H\n");
-    fprintf(stderr, "  -d, --debug N\t\t\tSet debug log level (0,1,2)\n");
+    fprintf(stderr, "  -d, --debug N\t\t\tSet debug log level (0,1,2,3)\n");
     fprintf(stderr, "  -v, --version\t\t\tPrint the version number\n");
     fprintf(stderr, "Report bugs to <%s>.\n",MAILING_LIST);
     exit(0);
   }
 
-  con_log_init(log_filename, log_level);
+  con_log_init(log_filename, loglevel);
   gamelog_init(gamelog_filename);
   gamelog_set_level(GAMELOG_FULL);
   gamelog(GAMELOG_NORMAL,"Starting new log");
diff -u -r --exclude-from exclude freeciv-cvs/server/console.c 
fc-adv/server/console.c
--- freeciv-cvs/server/console.c        Tue Apr 27 22:13:07 1999
+++ fc-adv/server/console.c     Sun May 30 12:35:17 1999
@@ -56,6 +56,7 @@
 void con_log_init(char *log_filename, int log_level)
 {
   log_init(log_filename, log_level, (log_filename ? NULL : con_handle_log));
+  logdebug_suppress_warning;
 }
 
 /************************************************************************