[syslog-ng] syslog-ng 2.0.x bug - facility(auth) fails to match any messages

[Balazs Scheidler]

On Fri, 2007-04-20 at 20:23 +0000, Balazs Scheidler wrote:
> On Mon, 2007-04-16 at 10:33 -0700, Evan Rempel wrote:
> > Balazs Scheidler wrote:
> 
> > and use filter rather than using two filters in the log line,
> > things work as documented.
> > 
> > The problem is with messages that should match the facility of auth not with
> > the messages that match local1 and ldap.
> > 
> > > 
> > > This means that:
> > > 
> > >> filter f_auth         { facility(auth); };
> > >> filter f_local1       { facility(local1); };
> > >> filter f_ldap       { program(^slapd); };
> > >>
> > >> # ****** PROBLEM LOG LINE ****
> > >> log { source(local); filter(f_local1); filter(f_ldap); destination(test.log); };
> > > 
> > > Test.log will only receive a message if the facility is local1 and the 
> > > program sending the message is slapd. I've googled for this and slapd uses 
> > > local4 by default:
> > 
> > Yes, in our environment our primary LDAP server logs with the facility of local1.
> > 
> > It is easy to test. Use the attached configuration and try to log with any priority auth.*
> > and see where the messages get logged. They should be logged in the auth.log but on my system they
> > are logged into unknown.log
> > 
> 
> Sorry for not responding for so long. The problem you reported was was
> indeed a bug in syslog-ng. The patch below fixes the problem in the code
> and adjusts the unit testcase to detect the problem.
> 
> As a side-note, I've migrated to use git instead of GNU arch, thus daily
> snapshots will not receive this and further updates until I fix that to
> use my git tree instead. So you need to apply this patch by hand.
> 
> Please give me some time to fix the snapshot generation. I'll try to
> publish my git tree in the interim time.
> 
> diff --git a/src/filter.c b/src/filter.c
> index 04e3639..269d016 100644
> --- a/src/filter.c
> +++ b/src/filter.c

this patch is crap, it fixes the specific problem, but may not cover
another case. Here's a better version instead.

diff --git a/src/cfg-grammar.y b/src/cfg-grammar.y
index 6f5cfe1..af729fd 100644
--- a/src/cfg-grammar.y
+++ b/src/cfg-grammar.y
@@ -808,7 +808,7 @@ filter_simple_expr
 	;
 
 filter_fac_list
-	: filter_fac filter_fac_list		{ $$ = $1 + $2; }
+	: filter_fac filter_fac_list		{ $$ = $1 | $2; }
 	| filter_fac				{ $$ = $1; }
 	;
 
@@ -830,7 +830,7 @@ filter_fac
 	;
 
 filter_level_list
-	: filter_level filter_level_list	{ $$ = $1 + $2; }
+	: filter_level filter_level_list	{ $$ = $1 | $2; }
 	| filter_level				{ $$ = $1; }
 	;
 
diff --git a/src/filter.c b/src/filter.c
index 04e3639..8e19440 100644
--- a/src/filter.c
+++ b/src/filter.c
@@ -178,45 +178,16 @@ static gboolean
 filter_facility_eval(FilterExprNode *s, LogMessage *msg)
 {
   FilterPri *self = (FilterPri *) s;
-  guint32 fac = msg->pri & LOG_FACMASK, fac_num = (msg->pri & LOG_FACMASK) >> 3;
-  guint32 bits = self->valid;
-  int i;
+  guint32 fac_num = (msg->pri & LOG_FACMASK) >> 3;
   
-  if (G_UNLIKELY(bits & 0x80000000))
+  if (G_UNLIKELY(self->valid & 0x80000000))
     {
       /* exact number specified */
-      return ((bits & ~0x80000000) == fac_num) ^ s->comp;
+      return ((self->valid & ~0x80000000) == fac_num) ^ s->comp;
     }
   else
     {
-      static gint8 bitpos[32] = 
-      { 
-        -1, -1, -1, -1, -1, -1, -1, -1, 
-        -1, -1, -1, -1, -1, -1, -1, -1, 
-        -1, -1, -1, -1, -1, -1, -1, -1, 
-        -1, -1, -1, -1, -1, -1, -1, -1
-      };
-      
-      if (G_LIKELY(fac_num < 32 && bitpos[fac_num] != -1))
-        {
-          if (self->valid & (1 << bitpos[fac_num]))
-            return !self->super.comp;
-        }
-      else
-        {
-          for (i = 0; bits && sl_facilities[i].name; i++, bits >>= 1) 
-            {
-              if (sl_facilities[i].value == fac) 
-                {
-                  if (fac_num < 32)
-                    bitpos[fac_num] = i;
-                  if (bits & 1)
-                    {
-                      return !self->super.comp;
-                    }
-                }
-            }
-        }
+      return !!(self->valid & (1 << fac_num)) ^ self->super.comp;
     }
   return self->super.comp;
 }
diff --git a/src/syslog-names.h b/src/syslog-names.h
index 5afc924..40e8b4f 100644
--- a/src/syslog-names.h
+++ b/src/syslog-names.h
@@ -43,7 +43,21 @@ const char *syslog_name_lookup_name_by_value(int value, struct sl_name names[]);
 
 guint32 syslog_make_range(guint32 r1, guint32 r2);
 
-#define syslog_name_lookup_level_by_name(name) syslog_name_lookup_value_by_name(name, sl_levels)
-#define syslog_name_lookup_facility_by_name(name) syslog_name_lookup_id_by_name(name, sl_facilities)
+static inline guint32
+syslog_name_lookup_level_by_name(const gchar *name) 
+{
+  return syslog_name_lookup_value_by_name(name, sl_levels);
+}
+
+static inline guint32
+syslog_name_lookup_facility_by_name(const gchar *name) 
+{
+  guint32 res;
+  
+  res = syslog_name_lookup_value_by_name(name, sl_facilities);
+  if (res != -1)
+    return res >> 3;
+  return res;
+}
 
 #endif


-- 
Bazsi