Bug 24218

Comment 1 dwmalone 2001-01-10 12:21:31 UTC

Responsible Changed
From-To: freebsd-bugs->dwmalone

I'll have a look at this one.

On Wed, Jan 10, 2001 at 11:12:21PM +1100, john@pakprotector.com wrote:

> >Synopsis:       Tcp_wrappers is missing some syslog facility names.

It turns out that syslog.h provides a magic way of doing this, if
you define SYSLOG_NAMES. This magic is already used by natd, syslogd
and logger.

The patch needed to make tcp wrappers use this magic seems remarkably 
simple and would mean that we don't need to make any changes to
the tcp wrappers code to keep up in future.

I wonder if Mark could review the change? The only disadvantage I
can see is that it takes the code off the vendor branch - but we'd
have to do that anyway to provide access to the new syslog facility
names.

        David.


Index: options.c
===================================================================
RCS file: /cvs/FreeBSD-CVS/src/contrib/tcp_wrappers/options.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 options.c
--- options.c	1999/03/14 17:13:19	1.1.1.1
+++ options.c	2001/01/13 01:12:02
@@ -41,6 +41,7 @@
 #include <netinet/in.h>
 #include <netdb.h>
 #include <stdio.h>
+#define SYSLOG_NAMES
 #include <syslog.h>
 #include <pwd.h>
 #include <grp.h>
@@ -435,110 +436,17 @@
 	tcpd_jump("memory allocation failure");
 }
 
- /*
-  * The severity option goes last because it comes with a huge amount of ugly
-  * #ifdefs and tables.
-  */
-
-struct syslog_names {
-    char   *name;
-    int     value;
-};
-
-static struct syslog_names log_fac[] = {
-#ifdef LOG_KERN
-    "kern", LOG_KERN,
-#endif
-#ifdef LOG_USER
-    "user", LOG_USER,
-#endif
-#ifdef LOG_MAIL
-    "mail", LOG_MAIL,
-#endif
-#ifdef LOG_DAEMON
-    "daemon", LOG_DAEMON,
-#endif
-#ifdef LOG_AUTH
-    "auth", LOG_AUTH,
-#endif
-#ifdef LOG_LPR
-    "lpr", LOG_LPR,
-#endif
-#ifdef LOG_NEWS
-    "news", LOG_NEWS,
-#endif
-#ifdef LOG_UUCP
-    "uucp", LOG_UUCP,
-#endif
-#ifdef LOG_CRON
-    "cron", LOG_CRON,
-#endif
-#ifdef LOG_LOCAL0
-    "local0", LOG_LOCAL0,
-#endif
-#ifdef LOG_LOCAL1
-    "local1", LOG_LOCAL1,
-#endif
-#ifdef LOG_LOCAL2
-    "local2", LOG_LOCAL2,
-#endif
-#ifdef LOG_LOCAL3
-    "local3", LOG_LOCAL3,
-#endif
-#ifdef LOG_LOCAL4
-    "local4", LOG_LOCAL4,
-#endif
-#ifdef LOG_LOCAL5
-    "local5", LOG_LOCAL5,
-#endif
-#ifdef LOG_LOCAL6
-    "local6", LOG_LOCAL6,
-#endif
-#ifdef LOG_LOCAL7
-    "local7", LOG_LOCAL7,
-#endif
-    0,
-};
-
-static struct syslog_names log_sev[] = {
-#ifdef LOG_EMERG
-    "emerg", LOG_EMERG,
-#endif
-#ifdef LOG_ALERT
-    "alert", LOG_ALERT,
-#endif
-#ifdef LOG_CRIT
-    "crit", LOG_CRIT,
-#endif
-#ifdef LOG_ERR
-    "err", LOG_ERR,
-#endif
-#ifdef LOG_WARNING
-    "warning", LOG_WARNING,
-#endif
-#ifdef LOG_NOTICE
-    "notice", LOG_NOTICE,
-#endif
-#ifdef LOG_INFO
-    "info", LOG_INFO,
-#endif
-#ifdef LOG_DEBUG
-    "debug", LOG_DEBUG,
-#endif
-    0,
-};
-
 /* severity_map - lookup facility or severity value */
 
 static int severity_map(table, name)
-struct syslog_names *table;
+CODE   *table;
 char   *name;
 {
-    struct syslog_names *t;
+    CODE *t;
 
-    for (t = table; t->name; t++)
-	if (STR_EQ(t->name, name))
-	    return (t->value);
+    for (t = table; t->c_name; t++)
+	if (STR_EQ(t->c_name, name))
+	    return (t->c_val);
     tcpd_jump("bad syslog facility or severity: \"%s\"", name);
     /* NOTREACHED */
 }
@@ -554,8 +462,8 @@
     char   *level = split_at(value, '.');
 
     allow_severity = deny_severity = level ?
-	severity_map(log_fac, value) | severity_map(log_sev, level) :
-	severity_map(log_sev, value);
+	severity_map(facilitynames, value) | severity_map(prioritynames, level)
+	: severity_map(prioritynames, value);
 }
 
 /* get_field - return pointer to next field in string */

> On Wed, Jan 10, 2001 at 11:12:21PM +1100, john@pakprotector.com wrote:
> 
> > >Synopsis:       Tcp_wrappers is missing some syslog facility names.
> 
> It turns out that syslog.h provides a magic way of doing this, if
> you define SYSLOG_NAMES. This magic is already used by natd, syslogd
> and logger.

Cool! :-)

> The patch needed to make tcp wrappers use this magic seems remarkably 
> simple and would mean that we don't need to make any changes to
> the tcp wrappers code to keep up in future.

Is this "mainstream" TCP_Wrappers code? Will the next import of TCPW
have it?

> I wonder if Mark could review the change? The only disadvantage I
> can see is that it takes the code off the vendor branch - but we'd
> have to do that anyway to provide access to the new syslog facility
> names.

Looks good to me, by a quick eyeball check! Want me to take it further
than that?

If the code is not "mainstream", then it may be an idea to try to sell
it to Wietse Venema, and get is back in the next vendor import.

M

> ===================================================================
> RCS file: /cvs/FreeBSD-CVS/src/contrib/tcp_wrappers/options.c,v

-- 
Mark Murray
Warning: this .sig is umop ap!sdn

> Is this "mainstream" TCP_Wrappers code? Will the next import of TCPW
> have it?

Nope - Is Wietse working on a new version? The only update to the
tcp wrappers I've seen in the last few years was the support for
IPv6.

> Looks good to me, by a quick eyeball check! Want me to take it further
> than that?

I'll do a buildworld with the patch, just to be careful.

> If the code is not "mainstream", then it may be an idea to try to sell
> it to Wietse Venema, and get is back in the next vendor import.

Unfortunately, the syslog.h magic seems to be a BSDism (and by
extension shows up in some Linux versions of syslog.h). It's doesn't
seem to show up in AIX, Solaris or get mentioned in the SUSv2.  I
guess Wietse wouldn't be interested as it's using a vendor specific
feature he's unlikely to want to take it into the generic code
base.

	David.

> > If the code is not "mainstream", then it may be an idea to try to sell
> > it to Wietse Venema, and get is back in the next vendor import.
> 
> Unfortunately, the syslog.h magic seems to be a BSDism (and by
> extension shows up in some Linux versions of syslog.h). It's doesn't
> seem to show up in AIX, Solaris or get mentioned in the SUSv2.  I
> guess Wietse wouldn't be interested as it's using a vendor specific
> feature he's unlikely to want to take it into the generic code
> base.

No problem. As long as it is as clean as possible from a diff(1)
point of view (it seems to be), then it should be OK! :-)

M
-- 
Mark Murray
Warning: this .sig is umop ap!sdn

Comment 6 dwmalone 2001-03-05 13:06:48 UTC

State Changed
From-To: open->closed

Fixed in -current adn RELENG_4.