You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by rj...@apache.org on 2007/11/20 21:29:28 UTC

svn commit: r596813 - in /tomcat/connectors/trunk/jk/native: apache-1.3/mod_jk.c apache-2.0/mod_jk.c common/jk_logger.h common/jk_util.c common/jk_util.h

Author: rjung
Date: Tue Nov 20 12:29:28 2007
New Revision: 596813

URL: http://svn.apache.org/viewvc?rev=596813&view=rev
Log:
Because of some changed indentations, this one is easier to read in full text,
not as a diff :(

1) Still use static length format string for subsecond resolution time
   stamp formats, otherwise we would have to allocate a corresponding
   buffer dynamically for each log line.
   In case sub second precision is used, we only allow a format string
   of JK_TIME_MAX_SIZE-1=63 characters after replacement of subsecond
   format character by the respective digit sequence.
   Without subsecond resolution we still allow longer format strings.
2) Since the length of the sub second format string is restricted,
   use static length for the char array log_fmt_subsec
3) Since everything is static length, we have no more dynamic allocation.
   Remove jk_free_time_fmt() and revert r596746.
4) Make length checks during strncpy more explicit.
5) Move logging of used log time stamp format into the util method.
   The logger is completely initialized at the time we call this
   method.


Modified:
    tomcat/connectors/trunk/jk/native/apache-1.3/mod_jk.c
    tomcat/connectors/trunk/jk/native/apache-2.0/mod_jk.c
    tomcat/connectors/trunk/jk/native/common/jk_logger.h
    tomcat/connectors/trunk/jk/native/common/jk_util.c
    tomcat/connectors/trunk/jk/native/common/jk_util.h

Modified: tomcat/connectors/trunk/jk/native/apache-1.3/mod_jk.c
URL: http://svn.apache.org/viewvc/tomcat/connectors/trunk/jk/native/apache-1.3/mod_jk.c?rev=596813&r1=596812&r2=596813&view=diff
==============================================================================
--- tomcat/connectors/trunk/jk/native/apache-1.3/mod_jk.c (original)
+++ tomcat/connectors/trunk/jk/native/apache-1.3/mod_jk.c Tue Nov 20 12:29:28 2007
@@ -2458,14 +2458,12 @@
     if (jkl && flp) {
         jkl->log = jk_log_to_file;
         jkl->level = conf->log_level;
-        jk_set_time_fmt(jkl, conf->stamp_format_string);
         jkl->logger_private = flp;
         flp->log_fd = conf->log_fd;
         conf->log = jkl;
+        jk_set_time_fmt(conf->log, conf->stamp_format_string);
         if (main_log == NULL)
             main_log = conf->log;
-        jk_log(conf->log, JK_LOG_DEBUG, "log time stamp format is '%s'",
-               conf->log->log_fmt);
         return;
     }
 
@@ -2944,7 +2942,6 @@
                 if (conf->uw_map)
                     uri_worker_map_free(&conf->uw_map, NULL);
             }
-            jk_free_time_fmt(conf->log);
             conf->was_initialized = JK_FALSE;
         }
         s = s->next;

Modified: tomcat/connectors/trunk/jk/native/apache-2.0/mod_jk.c
URL: http://svn.apache.org/viewvc/tomcat/connectors/trunk/jk/native/apache-2.0/mod_jk.c?rev=596813&r1=596812&r2=596813&view=diff
==============================================================================
--- tomcat/connectors/trunk/jk/native/apache-2.0/mod_jk.c (original)
+++ tomcat/connectors/trunk/jk/native/apache-2.0/mod_jk.c Tue Nov 20 12:29:28 2007
@@ -2330,7 +2330,6 @@
                 if (conf->uw_map)
                     uri_worker_map_free(&conf->uw_map, NULL);
             }
-            jk_free_time_fmt(conf->log);
             conf->was_initialized = JK_FALSE;
         }
         s = s->next;
@@ -2535,11 +2534,8 @@
 {
     /* hgomez@20070425 */
     /* Clean up pointer content */
-    if (d != NULL) {
-        jk_logger_t *l = *(jk_logger_t **)d;
-        jk_free_time_fmt(l);
-        l = NULL;
-    }
+    if (d != NULL)
+        *(jk_logger_t **)d = NULL;
 
     return APR_SUCCESS;
 }
@@ -2608,10 +2604,10 @@
     if (jkl && flp) {
         jkl->log = jk_log_to_file;
         jkl->level = conf->log_level;
-        jk_set_time_fmt(jkl, conf->stamp_format_string);
         jkl->logger_private = flp;
         flp->jklogfp = conf->jklogfp;
         conf->log = jkl;
+        jk_set_time_fmt(conf->log, conf->stamp_format_string);
         if (main_log == NULL) {
             main_log = conf->log;
 
@@ -2620,8 +2616,6 @@
             /* Also should we pass pointer (ie: main_log) or handle (*main_log) ? */
             apr_pool_cleanup_register(p, &main_log, jklog_cleanup, jklog_cleanup);
         }
-        jk_log(conf->log, JK_LOG_DEBUG, "log time stamp format is '%s'",
-               conf->log->log_fmt);
 
         return 0;
     }

Modified: tomcat/connectors/trunk/jk/native/common/jk_logger.h
URL: http://svn.apache.org/viewvc/tomcat/connectors/trunk/jk/native/common/jk_logger.h?rev=596813&r1=596812&r2=596813&view=diff
==============================================================================
--- tomcat/connectors/trunk/jk/native/common/jk_logger.h (original)
+++ tomcat/connectors/trunk/jk/native/common/jk_logger.h Tue Nov 20 12:29:28 2007
@@ -31,17 +31,19 @@
 {
 #endif
 
+#define JK_TIME_MAX_SIZE (64)
+
 typedef struct jk_logger jk_logger_t;
 struct jk_logger
 {
     void *logger_private;
     int level;
-    const char *log_fmt;        /* the configured timestamp format for logging */
-    const char *log_fmt_subsec; /* like log_fmt, but milli/micro seconds
-                                   marker replaced, because strftime() doesn't handle those */
-    int    log_fmt_type;        /* do we want milli or microseconds */
-    size_t log_fmt_offset;      /* at which position should we insert */
-    size_t log_fmt_size;        /* how long is this format string */
+    const char *log_fmt;                   /* the configured timestamp format for logging */
+    char log_fmt_subsec[JK_TIME_MAX_SIZE]; /* like log_fmt, but milli/micro seconds marker
+                                              replaced, because strftime() doesn't handle those */
+    int    log_fmt_type;                   /* do we want milli or microseconds */
+    size_t log_fmt_offset;                 /* at which position should we insert */
+    size_t log_fmt_size;                   /* how long is this format string */
 
     int (JK_METHOD * log) (jk_logger_t *l, int level, int used, char *what);
 

Modified: tomcat/connectors/trunk/jk/native/common/jk_util.c
URL: http://svn.apache.org/viewvc/tomcat/connectors/trunk/jk/native/common/jk_util.c?rev=596813&r1=596812&r2=596813&view=diff
==============================================================================
--- tomcat/connectors/trunk/jk/native/common/jk_util.c (original)
+++ tomcat/connectors/trunk/jk/native/common/jk_util.c Tue Nov 20 12:29:28 2007
@@ -137,17 +137,6 @@
 #define JK_TIME_SUBSEC_NONE   (0)
 #define JK_TIME_SUBSEC_MILLI  (1)
 #define JK_TIME_SUBSEC_MICRO  (2)
-/* The size of the longest format string we can handle,
- * including the terminating '\0'.
- */
-#define JK_TIME_MAX_FMT_SIZE  (64)
-/* We increase the maximum format string size by the biggest
- * size increment, that happens during replacement of a
- * sub second format character by it's zero digit string.
- * At the moment, this is
- * strlen(JK_TIME_PATTERN_MICRO)-strlen(JK_TIME_CONV_MICRO) = 6-2 = 4.
- */
-#define JK_TIME_MAX_SIZE      (JK_TIME_MAX_FMT_SIZE + 4)
 
 /* Visual C++ Toolkit 2003 support */
 #if defined (_MSC_VER) && (_MSC_VER == 1310)
@@ -371,14 +360,12 @@
  * the format string, and then pass it on the strftime.
  * In order to do that efficiently, we prepare a format string, that
  * already has placeholder digits for the sub second time stamp
- * and we save the position and resultion of this placeholder.
+ * and we save the position and time precision of this placeholder.
  */
 void jk_set_time_fmt(jk_logger_t *l, const char *jk_log_fmt)
 {
     if (l) {
         char *s;
-        char log_fmt_safe[JK_TIME_MAX_FMT_SIZE];
-        char *fmt;
 
         if (!jk_log_fmt) {
 #ifndef NO_GETTIMEOFDAY
@@ -390,74 +377,58 @@
         l->log_fmt_type = JK_TIME_SUBSEC_NONE;
         l->log_fmt_offset = 0;
         l->log_fmt_size = 0;
-        l->log_fmt_subsec = jk_log_fmt;
         l->log_fmt = jk_log_fmt;
 
-/* This has enough space for JK_TIME_MAX_FMT_SIZE chars plus the longest
- * sub second replacement we might do in the format string.
+/* Look for the first occurence of JK_TIME_CONV_MILLI */
+        if ((s = strstr(jk_log_fmt, JK_TIME_CONV_MILLI))) {
+            size_t offset = s - jk_log_fmt;
+            size_t len = strlen(JK_TIME_PATTERN_MILLI);
+
+/* If we don't have enough space in our fixed-length char array,
+ * we simply stick to the default format, ignoring JK_TIME_CONV_MILLI.
+ * Otherwise we replace the first occurence of JK_TIME_CONV_MILLI by JK_TIME_PATTERN_MILLI.
  */
-        fmt = (char *)malloc(JK_TIME_MAX_SIZE);
-        if (fmt) {
-/* Ignore all chars in format string after position JK_TIME_MAX_FMT_SIZE.
- * This is simply for truncating the jk_log_fmt input param without
- * destroying the original.
- */
-            strncpy(log_fmt_safe, jk_log_fmt, JK_TIME_MAX_FMT_SIZE);
-            log_fmt_safe[JK_TIME_MAX_FMT_SIZE-1] = '\0';
-            if ((s = strstr(log_fmt_safe, JK_TIME_CONV_MILLI))) {
-/* We will replace the first occurence of JK_TIME_CONV_MILLI by JK_TIME_PATTERN_MILLI. */
-                size_t offset = s - log_fmt_safe;
-                size_t len = strlen(JK_TIME_PATTERN_MILLI);
-
+            if (offset + len < JK_TIME_MAX_SIZE) {
                 l->log_fmt_type = JK_TIME_SUBSEC_MILLI;
                 l->log_fmt_offset = offset;
-/* fmt has enough space for the truncated original format string plus
- * replacement of JK_TIME_CONV_MILLI by JK_TIME_PATTERN_MILLI. */
-                strncpy(fmt, log_fmt_safe, offset);
-                strncpy(fmt + offset, JK_TIME_PATTERN_MILLI, len);
-                strncpy(fmt + offset + len,
+                strncpy(l->log_fmt_subsec, jk_log_fmt, offset);
+                strncpy(l->log_fmt_subsec + offset, JK_TIME_PATTERN_MILLI, len);
+                strncpy(l->log_fmt_subsec + offset + len,
                         s + strlen(JK_TIME_CONV_MILLI),
-                        JK_TIME_MAX_SIZE - offset - len);
-/* Now we put a stop mark into fmt to make it's length at most JK_TIME_MAX_SIZE-1
- * plus terminating '\0'. This should not be necessary, because we already terminated
- * log_fmt_safe, but it makes the code more robust.
+                        JK_TIME_MAX_SIZE - offset - len - 1);
+/* Now we put a stop mark into the string to make it's length at most JK_TIME_MAX_SIZE-1
+ * plus terminating '\0'.
  */
-                fmt[JK_TIME_MAX_SIZE-1] = '\0';
-                l->log_fmt_subsec = fmt;
-                l->log_fmt_size = strlen(fmt);
+                l->log_fmt_subsec[JK_TIME_MAX_SIZE-1] = '\0';
+                l->log_fmt_size = strlen(l->log_fmt_subsec);
             }
-            else if ((s = strstr(log_fmt_safe, JK_TIME_CONV_MICRO))) {
-/* We will replace the first occurence of JK_TIME_CONV_MICRO by JK_TIME_PATTERN_MICRO. */
-                size_t offset = s - log_fmt_safe;
-                size_t len = strlen(JK_TIME_PATTERN_MICRO);
-
+/* Look for the first occurence of JK_TIME_CONV_MICRO */
+        }
+        else if ((s = strstr(jk_log_fmt, JK_TIME_CONV_MICRO))) {
+            size_t offset = s - jk_log_fmt;
+            size_t len = strlen(JK_TIME_PATTERN_MICRO);
+
+/* If we don't have enough space in our fixed-length char array,
+ * we simply stick to the default format, ignoring JK_TIME_CONV_MICRO.
+ * Otherwise we replace the first occurence of JK_TIME_CONV_MICRO by JK_TIME_PATTERN_MICRO.
+ */
+            if (offset + len < JK_TIME_MAX_SIZE) {
                 l->log_fmt_type = JK_TIME_SUBSEC_MICRO;
                 l->log_fmt_offset = offset;
-/* fmt has enough space for the truncated original format string plus
- * replacement of JK_TIME_CONV_MICRO by JK_TIME_PATTERN_MICRO. */
-                strncpy(fmt, log_fmt_safe, offset);
-                strncpy(fmt + offset, JK_TIME_PATTERN_MICRO, len);
-                strncpy(fmt + offset + len,
+                strncpy(l->log_fmt_subsec, jk_log_fmt, offset);
+                strncpy(l->log_fmt_subsec + offset, JK_TIME_PATTERN_MICRO, len);
+                strncpy(l->log_fmt_subsec + offset + len,
                         s + strlen(JK_TIME_CONV_MICRO),
-                        JK_TIME_MAX_SIZE - offset - len);
-/* Now we put a stop mark into fmt to make it's length at most JK_TIME_MAX_SIZE-1
- * plus terminating '\0'. This should not be necessary, because we already terminated
- * log_fmt_safe, but it makes the code more robust.
+                        JK_TIME_MAX_SIZE - offset - len - 1);
+/* Now we put a stop mark into the string to make it's length at most JK_TIME_MAX_SIZE-1
+ * plus terminating '\0'.
  */
-                fmt[JK_TIME_MAX_SIZE-1] = '\0';
-                l->log_fmt_subsec = fmt;
-                l->log_fmt_size = strlen(fmt);
+                l->log_fmt_subsec[JK_TIME_MAX_SIZE-1] = '\0';
+                l->log_fmt_size = strlen(l->log_fmt_subsec);
             }
         }
-    }
-}
-
-void jk_free_time_fmt(jk_logger_t *l)
-{
-    if (l && l->log_fmt_subsec &&
-        l->log_fmt != l->log_fmt_subsec) {
-            free((void *)l->log_fmt_subsec);
-            l->log_fmt_subsec = l->log_fmt;
+        jk_log(l, JK_LOG_DEBUG, "Pre-processed log time stamp format is '%s'",
+               l->log_fmt_type == JK_TIME_SUBSEC_NONE ? l->log_fmt : l->log_fmt_subsec);
     }
 }
 
@@ -466,6 +437,11 @@
     time_t t;
     struct tm *tms;
     int done;
+/* We want to use a fixed maximum size buffer here.
+ * If we would dynamically adjust it to the real format
+ * string length, we could support longer format strings,
+ * but we would have to allocate and free for each log line.
+ */
     char log_fmt[JK_TIME_MAX_SIZE];
 
     if (!l || !l->log_fmt) {
@@ -485,6 +461,11 @@
         rc = gettimeofday(&tv, NULL);
 #endif
         if (rc == 0) {
+/* We need this subsec buffer, because we convert
+ * the integer with sprintf(), but we don't
+ * want to write the terminating '\0' into our
+ * final log format string.
+ */
             char subsec[7];
             t = tv.tv_sec;
             strncpy(log_fmt, l->log_fmt_subsec, l->log_fmt_size + 1);

Modified: tomcat/connectors/trunk/jk/native/common/jk_util.h
URL: http://svn.apache.org/viewvc/tomcat/connectors/trunk/jk/native/common/jk_util.h?rev=596813&r1=596812&r2=596813&view=diff
==============================================================================
--- tomcat/connectors/trunk/jk/native/common/jk_util.h (original)
+++ tomcat/connectors/trunk/jk/native/common/jk_util.h Tue Nov 20 12:29:28 2007
@@ -41,8 +41,6 @@
 
 void jk_set_time_fmt(jk_logger_t *l, const char *jk_log_fmt);
 
-void jk_free_time_fmt(jk_logger_t *l);
-
 int jk_parse_log_level(const char *level);
 
 int jk_open_file_logger(jk_logger_t **l, const char *file, int level);



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org