On Wed 17.Mar'10 at  4:20:18 +0100, Tamas TEVESZ wrote:

> Subject: [PATCH] Fold many functions into one
> 
> The result is not much more convoluted than the original was, but much 
> shorter.
> Several vararg macros were added -- no idea what !gcc compilers make of this.
> 
> The messages sent to these functions are inconsistent across the source tree.
> I have now decided that the logging function will add the final newline -
> messages will need to be modified accordingly.
> 
> I have no idea why the original implementation was as complicated and ugly as
> it was. My guess is that it was anticipated that these are be called from
> sighandlers, but why no snprintf when they are all stuffed up with 
> vsnprintf...
> 
> Still, the result is not worse in this regard either.
> ---
>  WINGs/WINGs/WUtil.h |   21 +++++--
>  WINGs/error.c       |  153 
> ++++++++++++---------------------------------------
>  2 files changed, 52 insertions(+), 122 deletions(-)

Clean and beautiful, I will apply it.

Having said that, I have a _unfinished_ patch (in fact two) to address 
this very same code duplication but I also wanted to get rid of lots of

#ifdef DEBUG
       printf(foo);
#endif       

by 

     printw(WDEBUG, foo);

but iirc this aspect could be improved (ergo, the 'unfinished' status).

The idea was to have a new function printw() which would need the 'level'
as its first argument, like in

   printw(WARN, foo);

or 

  printw(WDEBUG, foo);
  
and in this later case it would be compiled out if DEBUG was not defined.
Besides that, I wanted the WDEBUG case to print the line in which it appears,
but haven't done that in the patch (another unfinished aspect of it).

The second patch (omitted) is much bigger and converts wmaker to use
the new printw() function.

I am attaching my first patch just in case you want to see it and flame
me, but I will apply your patch regardless and think about those
DEBUG ifdefs later.


---8<---
>From f4bc62c25431457024771c435b1694eac27908af Mon Sep 17 00:00:00 2001
From: Carlos R. Mafra <[email protected]>
Date: Thu, 27 Aug 2009 19:54:30 +0200
Subject: [PATCH 1/2] Add a generic printw() function

The reason for this change is to avoid code duplication in
WINGs/error.c regarding the special functions to print
system messages --  wwarning(), wfatal(), wsyserror() and
wmessage().

It also allows one to replace blocks of

       printf("foo");

with

       printw(WDEBUG, "foo")

and therefore avoid the proliferation of #ifdefs in the code,
as they are "hidden" in one place.

There are several importance "levels" for each message, and they
must be inserted in the beginning of the function call. E.g.,
to print a 'warning' message now one does

       printw(WARN, foo);

instead of wwarning(foo), and printw(ERR, foo) instead of
wsyserror(foo) etc.

The possible levels and their old equivalents are,

       1. MESSAGE : wmessage())
       2. WARN    : wwarning())
       3. ERR     : wsyserror())
       4. ERRCODE : wsyserrorcode(), still not functional
       5. FATAL   : wfatal()
       6. WDEBUG  : to replace #ifdef DEBUG printf(foo) #endif
---
 WINGs/WINGs/WUtil.h |   14 +++--
 WINGs/error.c       |  130 ++++++++++++++------------------------------------
 2 files changed, 45 insertions(+), 99 deletions(-)

diff --git a/WINGs/WINGs/WUtil.h b/WINGs/WINGs/WUtil.h
index 7c3f2de..9284434 100644
--- a/WINGs/WINGs/WUtil.h
+++ b/WINGs/WINGs/WUtil.h
@@ -10,6 +10,13 @@
 #define NULL ((void*)0)
 #endif
 
+/* Linux-kernel-like print levels */
+#define MESSAGE 1
+#define WARN    2
+#define ERR     3
+#define ERRCODE 4
+#define FATAL   5
+#define WDEBUG  6
 
 #ifndef WMAX
 # define WMAX(a,b)     ((a)>(b) ? (a) : (b))
@@ -218,13 +225,8 @@ waborthandler* wsetabort(waborthandler* handler);
 
 /* don't free the returned string */
 char* wstrerror(int errnum);
-
-void wmessage(const char *msg, ...) __attribute__((__format__(printf,1,2)));
-void wwarning(const char *msg, ...) __attribute__((__format__(printf,1,2)));
-void wfatal(const char *msg, ...) __attribute__((__format__(printf,1,2)));
-void wsyserror(const char *msg, ...) __attribute__((__format__(printf,1,2)));
 void wsyserrorwithcode(int error, const char *msg, ...) 
__attribute__((__format__(printf,2,3)));
-
+void printw(const short level, const char *msg, ...) 
__attribute__((__format__(printf,2,3)));
 char* wfindfile(char *paths, char *file);
 
 char* wfindfileinlist(char **path_list, char *file);
diff --git a/WINGs/error.c b/WINGs/error.c
index e0aa4e8..b3ac0a8 100644
--- a/WINGs/error.c
+++ b/WINGs/error.c
@@ -19,6 +19,7 @@
  */
 
 #include "wconfig.h"
+#include "../WINGs/WINGs/WUtil.h"
 
 #include <stdio.h>
 #include <stdlib.h>
@@ -58,129 +59,72 @@ char *wstrerror(int errnum)
 }
 
 /*********************************************************************
- * Prints a message with variable arguments
+ * Prints a system error message with variable arguments, being given
+ * the error code.
  *
- * msg - message to print with optional formatting
- * ... - arguments to use on formatting
+ * error - the error code foe which to print the message
+ * msg   - message to print with optional formatting
+ * ...   - arguments to use on formatting
  *********************************************************************/
-void wmessage(const char *msg, ...)
+void wsyserrorwithcode(int error, const char *msg, ...)
 {
        va_list args;
        char buf[MAXLINE];
 
        va_start(args, msg);
-
        vsnprintf(buf, MAXLINE - 3, msg, args);
-       strcat(buf, "\n");
        fflush(stdout);
        fputs(_WINGS_progname ? _WINGS_progname : "WINGs", stderr);
-       fputs(": ", stderr);
+       fputs(_(" error: "), stderr);
        fputs(buf, stderr);
-       fflush(stdout);
+       fputs(": ", stderr);
+       fputs(wstrerror(error), stderr);
+       fputs("\n", stderr);
        fflush(stderr);
-
-       va_end(args);
-}
-
-/*********************************************************************
- * Prints a warning message with variable arguments
- *
- * msg - message to print with optional formatting
- * ... - arguments to use on formatting
- *********************************************************************/
-void wwarning(const char *msg, ...)
-{
-       va_list args;
-       char buf[MAXLINE];
-
-       va_start(args, msg);
-
-       vsnprintf(buf, MAXLINE - 3, msg, args);
-       strcat(buf, "\n");
-       fflush(stdout);
-       fputs(_WINGS_progname ? _WINGS_progname : "WINGs", stderr);
-       fputs(_(" warning: "), stderr);
-       fputs(buf, stderr);
        fflush(stdout);
-       fflush(stderr);
-
        va_end(args);
 }
 
-/**************************************************************************
- * Prints a fatal error message with variable arguments and terminates
+/*
+ * Prints system messages (warnings, errors, etc) with variable arguments
  *
+ * level - the importance level (MESSAGE, WARN, FATAL, ERR, WDEBUG)
  * msg - message to print with optional formatting
  * ... - arguments to use on formatting
- **************************************************************************/
-void wfatal(const char *msg, ...)
+ */
+void printw(const short level, const char *msg, ...)
 {
-       va_list args;
        char buf[MAXLINE];
-
-       va_start(args, msg);
-
-       vsnprintf(buf, MAXLINE - 3, msg, args);
-       strcat(buf, "\n");
-       fflush(stdout);
-       fputs(_WINGS_progname ? _WINGS_progname : "WINGs", stderr);
-       fputs(_(" fatal error: "), stderr);
-       fputs(buf, stderr);
-       fflush(stdout);
-       fflush(stderr);
-
-       va_end(args);
-}
-
-/*********************************************************************
- * Prints a system error message with variable arguments
- *
- * msg - message to print with optional formatting
- * ... - arguments to use on formatting
- *********************************************************************/
-void wsyserror(const char *msg, ...)
-{
        va_list args;
-       char buf[MAXLINE];
        int error = errno;
 
-       va_start(args, msg);
-       vsnprintf(buf, MAXLINE - 3, msg, args);
-       fflush(stdout);
+       /* All messages have a common initial beginning */
        fputs(_WINGS_progname ? _WINGS_progname : "WINGs", stderr);
-       fputs(_(" error: "), stderr);
-       fputs(buf, stderr);
-       fputs(": ", stderr);
-       fputs(wstrerror(error), stderr);
-       fputs("\n", stderr);
-       fflush(stderr);
-       fflush(stdout);
-       va_end(args);
-}
-
-/*********************************************************************
- * Prints a system error message with variable arguments, being given
- * the error code.
- *
- * error - the error code foe which to print the message
- * msg   - message to print with optional formatting
- * ...   - arguments to use on formatting
- *********************************************************************/
-void wsyserrorwithcode(int error, const char *msg, ...)
-{
-       va_list args;
-       char buf[MAXLINE];
 
+       if (level == MESSAGE)
+               fputs(" : ", stderr);
+       else if (level == WARN)
+               fputs(" warning: ", stderr);
+       else if (level == FATAL)
+               fputs(" fatal error: ", stderr);
+
+       else if (level == ERR) {
+               fputs(" error: ", stderr);
+               fputs(wstrerror(error), stderr);
+               fputs("\n", stderr);
+       }
+       else if (level == WDEBUG)
+#ifdef DEBUG
+               fputs(" DEBUG: ", stderr);
+#else
+       do {} while (0);
+#endif
        va_start(args, msg);
        vsnprintf(buf, MAXLINE - 3, msg, args);
+       strcat(buf, "\n");
        fflush(stdout);
-       fputs(_WINGS_progname ? _WINGS_progname : "WINGs", stderr);
-       fputs(_(" error: "), stderr);
        fputs(buf, stderr);
-       fputs(": ", stderr);
-       fputs(wstrerror(error), stderr);
-       fputs("\n", stderr);
-       fflush(stderr);
        fflush(stdout);
+       fflush(stderr);
        va_end(args);
 }
-- 
1.7.0.2.182.ge007


-- 
To unsubscribe, send mail to [email protected].

Reply via email to