Should UTMP be compiled by default now?

David Lee t.d.lee at durham.ac.uk
Tue Jun 3 10:18:12 GMT 2003


On Thu, 29 May 2003, David Lee wrote:

> On Sat, 24 May 2003 jra at dp.samba.org wrote:
>
> > On Sat, May 24, 2003 at 08:30:53PM +1000, Andrew Bartlett wrote:
> > > I was wondering - should we enable the use of utmp as a 'stable' feature
> > > for Samba 3.0, given it has been in Samba 2.2 with very few complaints
> > > for quite some time now.
> > >
> > > When compiled in, the utmp code must still be activated with a global
> > > 'utmp = yes' parameter.
> > >
> > > What do people think?
> >
> > Yes I don't see any harm in this. We should monitor the compile
> > problems people have during the betas though.
>
> For info to list folk: Andrew Bartlett has just (offlist) asked me (as
> original author and coordinator of the utmp code) to look at this.
>
> I have just sketched a possible patch to implement this, and am about to
> email it to Andrew to discuss (there are a couple of interactions with his
> "session" code).
> [...]

Further to my message above:

Over the last few days, and guided by Andrew B., I have produced the
attached patch.  It affects four files.  Overall it simplifies.
(Reduction in complexity: how often do we get that in these days of
manufacturer-led software bloat??)

1. configure.in:  Previously it would set "WITH_UTMP" to "no" unless
   explicitly overridden with "--with-utmp=yes".  Now it attempts to set
   it to "yes".  (It will only end up "no" if either there is no "utmp.h"
   file, or the configurer has explicitly requested "--with-utmp=no" (or
   its "--without-utmp" equivalent).

2. param/loadparm.c:  Used to have three "#ifdef WITH_UTMP", two of which
   would prevent "lp_utmp()" and its support being compiled in.  Those
   have now been removed.  Now "lp_utmp()" is always available.  Only the
   "#ifdef" which parses smb.conf's "utmp = true" remains (so those
   installations where utmp is unavailable (utmp.h or configure) have
   smb.conf "utmp" as invalid).

3. smbd/utmp.c: Previously, the entry points "sys_utmp_claim()" and
   "sys_utmp_yield()" were only valid under "WITH_UTMP".  Now they are
   always valid: in the case of no "WITH_UTMP" they are trivial,
   instant-return routines.

4. smbd/session.c:  Conceptually this is the "application" which uses the
   utmp "service".  Previously it had three "#ifdef WITH_UTMP".  But now,
   because "lp_utmp()", "sys_utmp_claim()" and "sys_utmp_yield()" are
   always valid, we have been able to remove these "#ifdef"s.

Comments?

I can see a possible further optimisation in "configure.in".  But I don't
want that to distract us from the overall principle.  So I mention it here
simply as a recognition and a place-holder.  For the moment, can we
concentrate on patch, as discussed above (and not be distracted too much
onto the optimisation below)?

At present "configure" does:
  1.  look for utmp.h
  2.  work out utmp details
  3.  process possible "--without-utmp" option

Now if explicit "--without-utmp" is true on a system with utmp, then the
earlier steps were wasted.  So in that sense it would be better to
reorder those steps:
  1. process possible "--without-utmp" option
  2. (conditionally) look for utmp.h
  3. (conditionally) work out utmp details

But there are counter arguments (admittedly not strong) for leaving as-is
(i.e. current, with attached patch):

  1. How many sites will really want explicit "--without-utmp" at compile
     time?  (Remember, configure-time will still detect lack of native
     utmp, and run-time will still default to "utmp = False".)

  2. It would make this patch bigger!  In the long run that's a poor
     argument, of course.  But for the moment, as we agree the overall
     principle, I think this is a detail we can recognise and postpone
     (even if only for a few days).


-- 

:  David Lee                                I.T. Service          :
:  Systems Programmer                       Computer Centre       :
:                                           University of Durham  :
:  http://www.dur.ac.uk/t.d.lee/            South Road            :
:                                           Durham                :
:  Phone: +44 191 334 2752                  U.K.                  :
-------------- next part --------------
--- configure.in.orig	Thu May 29 08:57:42 2003
+++ configure.in	Fri May 30 15:13:50 2003
@@ -1359,6 +1359,9 @@
     AC_DEFINE(HAVE_UTIMBUF,1,[Whether struct utimbuf is available])
 fi
 
+##############
+# Check utmp details, but only if our OS offers utmp.h
+if test x"$ac_cv_header_utmp_h" = x"yes"; then
 dnl  utmp and utmpx come in many flavours
 dnl  We need to check for many of them
 dnl  But we don't need to do each and every one, because our code uses
@@ -1476,6 +1479,9 @@
     AC_DEFINE(HAVE_UX_UT_SYSLEN,1,[Whether the utmpx struct has a property ut_syslen])
 fi 
 
+fi
+# end utmp details
+
 
 ICONV_LOCATION=standard
 LOOK_DIRS="/usr /usr/local /sw"
@@ -2888,19 +2894,35 @@
 # check for experimental utmp accounting
 
 AC_MSG_CHECKING(whether to support utmp accounting)
+WITH_UTMP=yes
 AC_ARG_WITH(utmp,
-[  --with-utmp             Include experimental utmp accounting (default=no)],
+[  --with-utmp             Include utmp accounting (default, if supported by OS)],
 [ case "$withval" in
-  yes)
-    AC_MSG_RESULT(yes)
-    AC_DEFINE(WITH_UTMP,1,[Whether to include experimental utmp accounting])
-    ;;
+  no)
+		WITH_UTMP=no
+		;;
   *)
-    AC_MSG_RESULT(no)
-    ;;
+		WITH_UTMP=yes
+                ;;
   esac ],
-  AC_MSG_RESULT(no)
 )
+
+# utmp requires utmp.h
+# Note similar check earlier, when checking utmp details.
+
+if test x"$WITH_UTMP" = x"yes" -a x"$ac_cv_header_utmp_h" = x"no"; then
+	utmp_no_reason=", no utmp.h on $host_os"
+	WITH_UTMP=no
+fi
+
+# Display test results
+
+if test x"$WITH_UTMP" = x"yes"; then
+        AC_MSG_RESULT(yes)
+	AC_DEFINE(WITH_UTMP,1,[Whether to include experimental utmp accounting])
+else
+        AC_MSG_RESULT(no$utmp_no_reason)
+fi
 
 #################################################
 # choose native language(s) of man pages
--- param/loadparm.c.orig	Thu May 29 08:57:46 2003
+++ param/loadparm.c	Mon Jun  2 17:22:01 2003
@@ -158,11 +158,9 @@
 	char *szAbortShutdownScript;
 	char *szWINSHook;
 	char *szWINSPartners;
-#ifdef WITH_UTMP
 	char *szUtmpDir;
 	char *szWtmpDir;
 	BOOL bUtmp;
-#endif
 	char *szSourceEnv;
 	char *szIdmapUID;
 	char *szIdmapGID;
@@ -1572,11 +1570,9 @@
 FN_GLOBAL_STRING(lp_piddir, &Globals.szPidDir)
 FN_GLOBAL_STRING(lp_mangling_method, &Globals.szManglingMethod)
 FN_GLOBAL_INTEGER(lp_mangle_prefix, &Globals.mangle_prefix)
-#ifdef WITH_UTMP
 FN_GLOBAL_STRING(lp_utmpdir, &Globals.szUtmpDir)
 FN_GLOBAL_STRING(lp_wtmpdir, &Globals.szWtmpDir)
 FN_GLOBAL_BOOL(lp_utmp, &Globals.bUtmp)
-#endif
 FN_GLOBAL_STRING(lp_rootdir, &Globals.szRootdir)
 FN_GLOBAL_STRING(lp_source_environment, &Globals.szSourceEnv)
 FN_GLOBAL_STRING(lp_defaultservice, &Globals.szDefaultService)
--- smbd/utmp.c.orig	Fri May 16 13:15:04 2003
+++ smbd/utmp.c	Fri May 30 15:22:57 2003
@@ -21,8 +21,6 @@
 
 #include "includes.h"
 
-#ifdef WITH_UTMP
-
 /****************************************************************************
 Reflect connection status in utmp/wtmp files.
 	T.D.Lee at durham.ac.uk  September 1999
@@ -110,6 +108,23 @@
 
 ****************************************************************************/
 
+#ifndef WITH_UTMP
+/*
+ * Not WITH_UTMP?  Simply supply dummy routines.
+ */
+
+void sys_utmp_claim(const char *username, const char *hostname, 
+		    struct in_addr *ipaddr,
+		    const char *id_str, int id_num)
+{}
+
+void sys_utmp_yield(const char *username, const char *hostname, 
+		    struct in_addr *ipaddr,
+		    const char *id_str, int id_num)
+{}
+
+#else /* WITH_UTMP */
+
 #include <utmp.h>
 
 #ifdef HAVE_UTMPX_H
@@ -571,6 +586,4 @@
 	sys_utmp_update(&u, hostname, True);
 }
 
-#else /* WITH_UTMP */
- void dummy_utmp(void) {}
-#endif
+#endif /* WITH_UTMP */
--- smbd/session.c.orig	Wed May 14 01:46:42 2003
+++ smbd/session.c	Tue Jun  3 09:06:11 2003
@@ -66,7 +66,6 @@
 	data.dptr = NULL;
 	data.dsize = 0;
 
-#if WITH_UTMP
 	if (lp_utmp()) {
 		for (i=1;i<MAX_SESSION_ID;i++) {
 			slprintf(keystr, sizeof(keystr)-1, "ID/%d", i);
@@ -84,7 +83,6 @@
 		slprintf(sessionid.id_str, sizeof(sessionid.id_str)-1, SESSION_UTMP_TEMPLATE, i);
 		tdb_store_flag = TDB_MODIFY;
 	} else
-#endif
 	{
 		slprintf(keystr, sizeof(keystr)-1, "ID/%lu/%u", 
 			 (long unsigned int)sys_getpid(), 
@@ -137,13 +135,11 @@
 		return False;
 	}
 
-#if WITH_UTMP	
 	if (lp_utmp()) {
 		sys_utmp_claim(sessionid.username, sessionid.hostname, 
 			       client_ip,
 			       sessionid.id_str, sessionid.id_num);
 	}
-#endif
 
 	vuser->session_keystr = strdup(keystr);
 	if (!vuser->session_keystr) {
@@ -181,13 +177,11 @@
 
 	SAFE_FREE(dbuf.dptr);
 
-#if WITH_UTMP	
 	if (lp_utmp()) {
 		sys_utmp_yield(sessionid.username, sessionid.hostname, 
 			       client_ip,
 			       sessionid.id_str, sessionid.id_num);
 	}
-#endif
 
 	smb_pam_close_session(sessionid.username, sessionid.id_str, sessionid.hostname);
 


More information about the samba-technical mailing list