Tune-Up for getpw... in 2.2.0-alpha1

Richard Bollinger rabollinger at home.com
Tue Dec 12 00:57:20 GMT 2000


Tracing smbd (CVS version 2.2.0-alpha1) reveals that connecting to a given
share results in numerous avoidable calls to getpwnam().  A connection from
my pc (P139) to a share called "tmp" under the userid "rab" resulted in 23
calls to getpwnam:
    2    P139
    1    RAB
    1    TMP
    1    Tmp
    3    p139
    9    rab
    3    root
    1    tmP
    2    tmp

The odd variations on "tmp" are the result of a mysterious call to
add_session_user() in smbd/service.c.  Here's a patch to knock that out
(apparently without any adverse functional effect):

diff -ur --exclude *~ --exclude *.orig ../source.ref/smbd/service.c
./smbd/service.c
--- ../source.ref/smbd/service.c Fri Nov 17 18:05:47 2000
+++ ./smbd/service.c Mon Dec 11 16:33:06 2000
@@ -281,8 +281,12 @@
  /* lowercase the user name */
  strlower(user);

+#if 1
+ DEBUG(2,("add_session_user(\"%s\") skipped\n",service));
+#else
  /* add it as a possible user name */
  add_session_user(service);
+#endif

  /* shall we let them in? */
  if (!authorise_login(snum,user,password,pwlen,&guest,&force,vuid)) {


The odd variations on "p139" are the result of a mysterious call to
add_session_user() in smbd/reply.c.  Here's a patch to knock that out
(apparently also without any adverse functional effect):

diff -ur --exclude *~ --exclude *.orig ../source.ref/smbd/reply.c
./smbd/reply.c
--- ../source.ref/smbd/reply.c Mon Dec 11 15:21:11 2000
+++ ./smbd/reply.c Mon Dec 11 16:32:23 2000
@@ -137,7 +137,11 @@
  break;
  }

+#if 1
+ DEBUG(2,("add_session_user(\"%s\") skipped\n",remote_machine));
+#else
  add_session_user(remote_machine);
+#endif

  reload_services(True);
  reopen_logs();

Finally, here's a patch to lib/system.c to cache the most recent results
from a call to getpwnam() or getpwuid() to remove the last redundant calls:

diff -ur --exclude *~ --exclude *.orig ../source.ref/lib/system.c
./lib/system.c
--- ../source.ref/lib/system.c Sun Oct  8 17:21:24 2000
+++ ./lib/system.c Mon Dec 11 16:20:26 2000
@@ -608,13 +608,37 @@
  return &pw_ret;
 }

+static pstring sv_pw_name;
+static pstring sv_pw_passwd;
+static struct passwd sv_pw_ret;
+static int sv_valid = False;
+
 /**************************************************************************
  Wrapper for getpwnam(). Always returns a static that can be modified.

****************************************************************************
/

 struct passwd *sys_getpwnam(const char *name)
 {
- return setup_pwret(getpwnam(name));
+  struct passwd *ret;
+
+  if (!name || !name[0]) return NULL;
+  if (sv_valid && sv_pw_ret.pw_name && !strcmp(name, sv_pw_name)) {
+    DEBUG(2,("getpwnam(%s) avoided - using cached results\n",name));
+    return setup_pwret(&sv_pw_ret);
+  }
+  DEBUG(2,("getpwnam(%s) called\n",name));
+  if (!(ret = getpwnam(name))) return NULL;
+  memcpy((char *)&sv_pw_ret, ret, sizeof(struct passwd));
+  if (ret->pw_name) {
+    sv_pw_ret.pw_name = sv_pw_name;
+    pstrcpy(sv_pw_ret.pw_name, ret->pw_name);
+  }
+  if (ret->pw_passwd) {
+    sv_pw_ret.pw_passwd = sv_pw_passwd;
+    pstrcpy(sv_pw_ret.pw_passwd, ret->pw_passwd);
+  }
+  sv_valid = True;
+  return setup_pwret(&sv_pw_ret);
 }

 /**************************************************************************
@@ -623,7 +647,25 @@

 struct passwd *sys_getpwuid(uid_t uid)
 {
- return setup_pwret(getpwuid(uid));
+  struct passwd *ret;
+
+  if (sv_valid && uid == sv_pw_ret.pw_uid) {
+    DEBUG(2,("getpwuid(%d) avoided - using cached results\n",uid));
+    return setup_pwret(&sv_pw_ret);
+  }
+  DEBUG(2,("getpwuid(%d) called\n",uid));
+  if (!(ret = getpwuid(uid))) return NULL;
+  memcpy((char *)&sv_pw_ret, ret, sizeof(struct passwd));
+  if (ret->pw_name) {
+    sv_pw_ret.pw_name = sv_pw_name;
+    pstrcpy(sv_pw_ret.pw_name, ret->pw_name);
+  }
+  if (ret->pw_passwd) {
+    sv_pw_ret.pw_passwd = sv_pw_passwd;
+    pstrcpy(sv_pw_ret.pw_passwd, ret->pw_passwd);
+  }
+  sv_valid = True;
+  return setup_pwret(&sv_pw_ret);
 }

 /**************************************************************************

With these three patches in place, the same exercise as before results in
one call each to getpwnam() with the strings "root", "RAB" and "rab".  That
call for "RAB" might have also been avoided by reversing the default lookup
order for unix platforms, the most common case.  Perhaps someone knows why
those needless calls to add_session_user() were inserted in the first place?

Rich Bollinger, Elliott Company





More information about the samba-technical mailing list