Re: [Patch] Improve logging function

Jacek Raczkiewicz <[email protected]>
Newsgroups gmane.comp.mobile.kannel.devel
Message-ID <CABsGpivre7nhKrmnc+VvOp2PRKAZhj7aTMibRQ6hoJ=enLJmpw@mail.gmail.com>
Hi,

Ok I have fixed my patch and attached in the email.
I miss-understood the locking here, mea culpa.

Jacek


2013/10/3 Alexander Malysh <[email protected]>

> Hi,
>
> for improvement +1 for the implementation -1. You change a way how we used
> rwlock . Rwlock is to protect
> log file list and not the output. Please keep rwlock in place then +1 from
> me.
>
> Alex
>
> Am 02.10.2013 um 17:52 schrieb Jacek Raczkiewicz <
> [email protected]>:
>
> > Hi,
> >
> > I have been working on imrpving application that uses a lot libraries
> from kannel/gwlib.
> > I noticed that logging function does format the string even if there is
> no intention to save it to the log due to configured log level.
> > My improvement is that I moved the format() function inside the
> conditional block which check for log level.
> > This really make a difference when logged string is large (in our case
> it may be as big as 1MB) and that is formatted for no reason.
> >
> > Please include this fix into kannel codebase.
> >
> > Regards
> > Jacek Raczkiewicz
> > <logging.diff>
>
>
new_logging.diff (application/octet-stream, 2.1 KB)
*** kannel_fromSVN/gwlib/log.c	2013-10-02 11:02:28.288291416 -0400
--- kannel/gwlib/log.c	2013-10-04 03:03:50.052058929 -0400
***************
*** 511,522 ****
  	    char buf[FORMAT_SIZE]; \
  	    va_list args; \
  	    \
- 	    format(buf, level, place, err, fmt, 1); \
              gw_rwlock_rdlock(&rwlock); \
  	    for (i = 0; i < num_logfiles; ++i) { \
  		if (logfiles[i].exclusive == GW_NON_EXCL && \
                      level >= logfiles[i].minimum_output_level && \
                      logfiles[i].file != NULL) { \
  		        va_start(args, fmt); \
  		        output(logfiles[i].file, buf, args); \
  		        va_end(args); \
--- 513,524 ----
  	    char buf[FORMAT_SIZE]; \
  	    va_list args; \
  	    \
              gw_rwlock_rdlock(&rwlock); \
  	    for (i = 0; i < num_logfiles; ++i) { \
  		if (logfiles[i].exclusive == GW_NON_EXCL && \
                      level >= logfiles[i].minimum_output_level && \
                      logfiles[i].file != NULL) { \
+                 	format(buf, level, place, err, fmt, 1); \
  		        va_start(args, fmt); \
  		        output(logfiles[i].file, buf, args); \
  		        va_end(args); \
***************
*** 536,546 ****
  	    char buf[FORMAT_SIZE]; \
  	    va_list args; \
  	    \
- 	    format(buf, level, place, err, fmt, 1); \
              gw_rwlock_rdlock(&rwlock); \
              if (logfiles[e].exclusive == GW_EXCL && \
                  level >= logfiles[e].minimum_output_level && \
                  logfiles[e].file != NULL) { \
                  va_start(args, fmt); \
                  output(logfiles[e].file, buf, args); \
                  va_end(args); \
--- 538,548 ----
  	    char buf[FORMAT_SIZE]; \
  	    va_list args; \
  	    \
              gw_rwlock_rdlock(&rwlock); \
              if (logfiles[e].exclusive == GW_EXCL && \
                  level >= logfiles[e].minimum_output_level && \
                  logfiles[e].file != NULL) { \
+                 format(buf, level, place, err, fmt, 1); \
                  va_start(args, fmt); \
                  output(logfiles[e].file, buf, args); \
                  va_end(args); \
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.