On Tue, Jul 01, 2008 at 03:11:26PM -0700, Steven Dake wrote:
> Dave,
>
> Your patch looks reasonable but has a few issues which need to be
> addressed.
>
> It doesn't address the setting of logsys_subsys_id but defines it. I
> want to avoid the situation where logsys_subsys_id is defined, but then
> not set. What I suggest here is to set logsys_subsys_id to some known
> value (-1) and assert if that the subsystem id is that value within
> log_printf to help developers catch this scenario. At the moment the
> current API enforces proper behavior (it wont link if the developer does
> the wrong thing). With your patch it will link, but may not behave
> properly sending log messages to the wrong subsystem (0) instead of the
> subsystem desired by the developer. This is why the macros are there
> (to set the subsystem id and define it). Your patch addresses the
> removal of the definition to a generic location but doesn't address at
> all the setting of the subsystem id.
Good thought, done.
> The logsys_exit function doesn't need to be added. Instead this is
> managed by logsys_atsegv and logsys_flush. If you desire, you can keep
> logsys_exit and have it call logsys_flush which is the proper thing to
> do on exit.
OK, added flush to exit, and reset logsys_subsys_id back to -1.
> Please follow the coding style guidelines (ie: match the rest of the
> logsys code) so I don't have to rework your patch before commit.
OK, new patch attached.
Dave
Index: logsys.c
===================================================================
--- logsys.c (revision 1568)
+++ logsys.c (working copy)
@@ -632,3 +632,41 @@
{
worker_thread_group_wait (&log_thread_group);
}
+
+int logsys_init (char *name, int mode, int facility, int priority, char *file)
+{
+ char *errstr;
+
+ logsys_subsys_id = 0;
+
+ strncpy (logsys_loggers[0].subsys, name,
+ sizeof (logsys_loggers[0].subsys));
+ logsys_config_mode_set (mode);
+ logsys_config_facility_set (name, facility);
+ logsys_config_file_set (&errstr, file);
+ _logsys_config_priority_set (0, priority);
+ if ((mode & LOG_MODE_BUFFER_BEFORE_CONFIG) == 0) {
+ _logsys_wthread_create ();
+ }
+ return (0);
+}
+
+int logsys_conf (char *name, int mode, int facility, int priority, char *file)
+{
+ char *errstr;
+
+ strncpy (logsys_loggers[0].subsys, name,
+ sizeof (logsys_loggers[0].subsys));
+ logsys_config_mode_set (mode);
+ logsys_config_facility_set (name, facility);
+ logsys_config_file_set (&errstr, file);
+ _logsys_config_priority_set (0, priority);
+ return (0);
+}
+
+void logsys_exit (void)
+{
+ logsys_subsys_id = -1;
+ logsys_flush ();
+}
+
Index: logsys.h
===================================================================
--- logsys.h (revision 1568)
+++ logsys.h (working copy)
@@ -170,8 +170,9 @@
} \
}
+static unsigned int logsys_subsys_id __attribute__((unused)) = -1; \
+
#define LOGSYS_DECLARE_NOSUBSYS(priority) \
-static unsigned int logsys_subsys_id __attribute__((unused)); \
__attribute__ ((constructor)) static void logsys_nosubsys_init (void) \
{ \
_logsys_nosubsys_set(); \
@@ -180,7 +181,6 @@
}
#define LOGSYS_DECLARE_SUBSYS(subsys,priority) \
-static unsigned int logsys_subsys_id __attribute__((unused)); \
__attribute__ ((constructor)) static void logsys_subsys_init (void) \
{ \
logsys_subsys_id = \
@@ -188,6 +188,7 @@
}
#define log_printf(lvl, format, args...) do { \
+ assert (logsys_subsys_id != -1); \
if ((lvl) <= logsys_loggers[logsys_subsys_id].priority) { \
_logsys_log_printf2 (__FILE__, __LINE__, lvl, \
logsys_subsys_id, (format), ##args); \
@@ -195,6 +196,7 @@
} while(0)
#define dprintf(format, args...) do { \
+ assert (logsys_subsys_id != -1); \
if (LOG_LEVEL_DEBUG <= logsys_loggers[logsys_subsys_id].priority) { \
_logsys_log_printf2 (__FILE__, __LINE__, LOG_DEBUG, \
logsys_subsys_id, (format), ##args); \
@@ -202,6 +204,7 @@
} while(0)
#define ENTER_VOID() do { \
+ assert (logsys_subsys_id != -1); \
if (LOG_LEVEL_DEBUG <= logsys_loggers[logsys_subsys_id].priority) { \
_logsys_trace (__FILE__, __LINE__, LOGSYS_TAG_ENTER, \
logsys_subsys_id, ">%s\n", __FUNCTION__); \
@@ -209,6 +212,7 @@
} while(0)
#define ENTER(format, args...) do { \
+ assert (logsys_subsys_id != -1); \
if (LOG_LEVEL_DEBUG <= logsys_loggers[logsys_subsys_id].priority) { \
_logsys_trace (__FILE__, __LINE__, LOGSYS_TAG_ENTER, \
logsys_subsys_id, ">%s: " format, __FUNCTION__, \
@@ -217,6 +221,7 @@
} while(0)
#define LEAVE_VOID() do { \
+ assert (logsys_subsys_id != -1); \
if (LOG_LEVEL_DEBUG <= logsys_loggers[logsys_subsys_id].priority) { \
_logsys_trace (__FILE__, __LINE__, LOGSYS_TAG_LEAVE, \
logsys_subsys_id, "<%s\n", __FUNCTION__); \
@@ -224,6 +229,7 @@
} while(0)
#define LEAVE(format, args...) do { \
+ assert (logsys_subsys_id != -1); \
if (LOG_LEVEL_DEBUG <= logsys_loggers[logsys_subsys_id].priority) { \
_logsys_trace (__FILE__, __LINE__, LOGSYS_TAG_LEAVE, \
logsys_subsys_id, "<%s: " format, \
@@ -232,6 +238,7 @@
} while(0)
#define TRACE1(format, args...) do { \
+ assert (logsys_subsys_id != -1); \
if (LOG_LEVEL_DEBUG <= logsys_loggers[logsys_subsys_id].priority) { \
_logsys_trace (__FILE__, __LINE__, LOGSYS_TAG_TRACE1, \
logsys_subsys_id, (format), ##args); \
@@ -239,6 +246,7 @@
} while(0)
#define TRACE2(format, args...) do { \
+ assert (logsys_subsys_id != -1); \
if (LOG_LEVEL_DEBUG <= logsys_loggers[logsys_subsys_id].priority) { \
_logsys_trace (__FILE__, __LINE__, LOGSYS_TAG_TRACE2, \
logsys_subsys_id, (format), ##args); \
@@ -246,6 +254,7 @@
} while(0)
#define TRACE3(format, args...) do { \
+ assert (logsys_subsys_id != -1); \
if (LOG_LEVEL_DEBUG <= logsys_loggers[logsys_subsys_id].priority) { \
_logsys_trace (__FILE__, __LINE__, LOGSYS_TAG_TRACE3, \
logsys_subsys_id, (format), ##args); \
@@ -253,6 +262,7 @@
} while(0)
#define TRACE4(format, args...) do { \
+ assert (logsys_subsys_id != -1); \
if (LOG_LEVEL_DEBUG <= logsys_loggers[logsys_subsys_id].priority) { \
_logsys_trace (__FILE__, __LINE__, LOGSYS_TAG_TRACE4, \
logsys_subsys_id, (format), ##args); \
@@ -260,6 +270,7 @@
} while(0)
#define TRACE5(format, args...) do { \
+ assert (logsys_subsys_id != -1); \
if (LOG_LEVEL_DEBUG <= logsys_loggers[logsys_subsys_id].priority) { \
_logsys_trace (__FILE__, __LINE__, LOGSYS_TAG_TRACE5, \
logsys_subsys_id, (format), ##args); \
@@ -267,6 +278,7 @@
} while(0)
#define TRACE6(format, args...) do { \
+ assert (logsys_subsys_id != -1); \
if (LOG_LEVEL_DEBUG <= logsys_loggers[logsys_subsys_id].priority) { \
_logsys_trace (__FILE__, __LINE__, LOGSYS_TAG_TRACE6, \
logsys_subsys_id, (format), ##args); \
@@ -274,6 +286,7 @@
} while(0)
#define TRACE7(format, args...) do { \
+ assert (logsys_subsys_id != -1); \
if (LOG_LEVEL_DEBUG <= logsys_loggers[logsys_subsys_id].priority) { \
_logsys_trace (__FILE__, __LINE__, LOGSYS_TAG_TRACE7, \
logsys_subsys_id, (format), ##args); \
@@ -281,6 +294,7 @@
} while(0)
#define TRACE8(format, args...) do { \
+ assert (logsys_subsys_id != -1); \
if (LOG_LEVEL_DEBUG <= logsys_loggers[logsys_subsys_id].priority) { \
_logsys_trace (__FILE__, __LINE__, LOGSYS_TAG_TRACE8, \
logsys_subsys_id, (format), ##args); \
@@ -293,4 +307,10 @@
_logsys_config_priority_set (logsys_subsys_id, priority); \
} while(0)
+/* simple, function-based api */
+
+int logsys_init (char *name, int mode, int facility, int priority, char *file);
+int logsys_conf (char *name, int mode, int facility, int priority, char *file);
+void logsys_exit (void);
+
#endif /* LOGSYS_H_DEFINED */
_______________________________________________
Openais mailing list
Openais@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/openais
