NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: lib/40728: expanding NIS-Netgroups for users and groups is very slow



The following reply was made to PR lib/40728; it has been noted by GNATS.

From: Wolfgang Stukenbrock <wolfgang.stukenbrock%nagler-company.com@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc: 
Subject: Re: lib/40728: expanding NIS-Netgroups for users and groups is very 
slow
Date: Thu, 29 Mar 2012 11:24:31 +0200

 This is a multi-part message in MIME format.
 --------------040207020604010308030301
 Content-Type: text/plain; charset=us-ascii; format=flowed
 Content-Transfer-Encoding: 7bit
 
 Hi again,
 
 I've now switched my passwd database setup from passwd to master.passwd
 YP-maps and recognised a bug in my patch.
 
 Due to the fact that _nis_start() is no longer called prior calling
 _nis_pwscan in any case, we may fail to select the master.passwd map
 correctly.
 
 Due to the fact that we should not call _nis_start() if we reuse the
 state, there is no easy way to add this call again.
 The better sollution - from my point of view - is to delay the
 map-selection in _nis_pwscan() after the (optional) build-in call to
 _nis_start.
 
 The following patch will solve this problem by passing an array instead
 of the map name and take the correct map from there after the optional
 _nis_start() call in _nis_pwscan:
 
 --- getpwent.c  2010/05/03 12:54:18     1.2
 +++ getpwent.c  2012/03/28 11:29:48
 @@ -1099,7 +1099,7 @@
           char            *current;       /* current first/next match */
           int              currentlen;    /* length of _nis_current */
           enum {                          /* shadow map type */
 -               NISMAP_UNKNOWN,         /*  unknown ... */
 +               NISMAP_UNKNOWN = 0,     /*  unknown ... */
                   NISMAP_NONE,            /*  none: use "passwd.by*" */
                   NISMAP_ADJUNCT,         /*  pw_passwd from
 "passwd.adjunct.*" */
                   NISMAP_MASTER           /*  all from 
 "master.passwd.by*" */
 @@ -1111,11 +1111,17 @@
    static struct passwd           _nis_passwd;
    static char                    _nis_passwdbuf[_GETPW_R_SIZE_MAX];
 
 +static const char __nis_pw_n_1[] = "master.passwd.byname";
 +static const char __nis_pw_n_2[] = "passwd.byname";
 +static const char __nis_pw_u_1[] = "master.passwd.byuid";
 +static const char __nis_pw_u_2[] = "passwd.byuid";
 +
 +static const char * const __nis_pw_n_map[4] = { __nis_pw_n_2,
 __nis_pw_n_2, __nis_pw_n_2, __nis_pw_n_1 };
 +static const char * const __nis_pw_u_map[4] = { __nis_pw_u_2,
 __nis_pw_u_2, __nis_pw_u_2, __nis_pw_u_1 };
 +
           /* macros for deciding which NIS maps to use. */
 -#define        PASSWD_BYNAME(x)        ((x)->maptype == NISMAP_MASTER \
 -                                   ? "master.passwd.byname" :
 "passwd.byname")
 -#define        PASSWD_BYUID(x)         ((x)->maptype == NISMAP_MASTER \
 -                                   ? "master.passwd.byuid" :
 "passwd.byuid")
 +#define        PASSWD_BYNAME(x)        ((x)->maptype == NISMAP_MASTER ?
 __nis_pw_n_1 : __nis_pw_n_2)
 +#define        PASSWD_BYUID(x)         ((x)->maptype == NISMAP_MASTER ?
 __nis_pw_u_1 : __nis_pw_u_2)
 
    static int
    _nis_start(struct nis_state *state)
 @@ -1236,7 +1242,7 @@
     */
    static int
    _nis_pwscan(int *retval, struct passwd *pw, char *buffer, size_t buflen,
 -       struct nis_state *state, const char *map)
 +       struct nis_state *state, const char * const *map_arr)
    {
           char    *data;
           int     nisr, rv, datalen;
 @@ -1245,7 +1251,7 @@
           _DIAGASSERT(pw != NULL);
           _DIAGASSERT(buffer != NULL);
           _DIAGASSERT(state != NULL);
 -       _DIAGASSERT(map != NULL);
 +       _DIAGASSERT(map_arr != NULL);
 
           *retval = 0;
 
 @@ -1257,9 +1263,10 @@
 
           data = NULL;
           rv = NS_NOTFOUND;
 +       _DIAGASSERT(state->maptype > 0 && state->maptype <
 sizeof(map_arr)/sizeof(const char*));
 
                                                           /* search map */
 -       nisr = yp_match(state->domain, map, buffer, (int)strlen(buffer),
 +       nisr = yp_match(state->domain, map_arr[state->maptype], buffer,
 (int)strlen(buffer),
               &data, &datalen);
           switch (nisr) {
           case 0:
 @@ -1494,7 +1501,7 @@
           snprintf(_nis_passwdbuf, sizeof(_nis_passwdbuf), "%u",
 (unsigned int)uid);
           rv = _nis_pwscan(&rerror, &_nis_passwd,
               _nis_passwdbuf, sizeof(_nis_passwdbuf),
 -           &_nis_state, PASSWD_BYUID(&_nis_state));
 +           &_nis_state, __nis_pw_u_map);
           if (!_nis_state.stayopen)
                   _nis_end(&_nis_state);
           if (rv == NS_SUCCESS && uid == _nis_passwd.pw_uid)
 @@ -1527,14 +1534,14 @@
           if (_nis_state.stayopen)
             { /* use global state only if stayopen is set - otherwiese we
 would blow up getpwent_r() ... */
               rv = _nis_pwscan(retval, pw, buffer, buflen,
 -               &_nis_state, PASSWD_BYUID(&_nis_state));
 +               &_nis_state, __nis_pw_u_map);
             }
           else
             { /* keep old semantic if no stayopen set - no need to call
 _nis_start() here - _nis_pwscan() will do it for us ... */
               /* use same way as in getgrent.c ... */
               memset(&state, 0, sizeof(state));
               rv = _nis_pwscan(retval, pw, buffer, buflen,
 -               &state, PASSWD_BYUID(&state));
 +               &state, __nis_pw_u_map);
               _nis_end(&state);
             }
           if (rv != NS_SUCCESS)
 @@ -1564,7 +1571,7 @@
           snprintf(_nis_passwdbuf, sizeof(_nis_passwdbuf), "%s", name);
           rv = _nis_pwscan(&rerror, &_nis_passwd,
               _nis_passwdbuf, sizeof(_nis_passwdbuf),
 -           &_nis_state, PASSWD_BYNAME(&_nis_state));
 +           &_nis_state, __nis_pw_n_map);
           if (!_nis_state.stayopen)
                   _nis_end(&_nis_state);
           if (rv == NS_SUCCESS && strcmp(name, _nis_passwd.pw_name) == 0)
 @@ -1597,14 +1604,14 @@
           if (_nis_state.stayopen)
             { /* use global state only if stayopen is set - otherwiese we
 would blow up getpwent_r() ... */
               rv = _nis_pwscan(retval, pw, buffer, buflen,
 -               &_nis_state, PASSWD_BYNAME(&_nis_state));
 +               &_nis_state, __nis_pw_n_map);
             }
           else
             { /* keep old semantic if no stayopen set - no need to call
 _nis_start() here - _nis_pwscan() will do it for us ... */
               /* use same way as in getgrent.c ... */
               memset(&state, 0, sizeof(state));
               rv = _nis_pwscan(retval, pw, buffer, buflen,
 -               &state, PASSWD_BYNAME(&state));
 +               &state, __nis_pw_n_map);
               _nis_end(&state);
             }
           if (rv != NS_SUCCESS)
 
 
 
 ###### end of additional patch ##########
 
 
 And for more easy integration the complete patch for getpwent.c against
 5.1-release again as attachment ....
 
 
 
 --------------040207020604010308030301
 Content-Type: text/plain;
  name="getpwent-1.c-patch"
 Content-Transfer-Encoding: 7bit
 Content-Disposition: inline;
  filename="getpwent-1.c-patch"
 
 --- getpwent.c 2010/05/03 12:50:13     1.1
 +++ getpwent.c 2012/03/28 11:29:48
 @@ -1099,7 +1099,7 @@
        char            *current;       /* current first/next match */
        int              currentlen;    /* length of _nis_current */
        enum {                          /* shadow map type */
 -              NISMAP_UNKNOWN,         /*  unknown ... */
 +              NISMAP_UNKNOWN = 0,     /*  unknown ... */
                NISMAP_NONE,            /*  none: use "passwd.by*" */
                NISMAP_ADJUNCT,         /*  pw_passwd from "passwd.adjunct.*" */
                NISMAP_MASTER           /*  all from "master.passwd.by*" */
 @@ -1111,11 +1111,17 @@
  static struct passwd          _nis_passwd;
  static char                   _nis_passwdbuf[_GETPW_R_SIZE_MAX];
  
 +static const char __nis_pw_n_1[] = "master.passwd.byname";
 +static const char __nis_pw_n_2[] = "passwd.byname";
 +static const char __nis_pw_u_1[] = "master.passwd.byuid";
 +static const char __nis_pw_u_2[] = "passwd.byuid";
 +
 +static const char * const __nis_pw_n_map[4] = { __nis_pw_n_2, __nis_pw_n_2, 
__nis_pw_n_2, __nis_pw_n_1 };
 +static const char * const __nis_pw_u_map[4] = { __nis_pw_u_2, __nis_pw_u_2, 
__nis_pw_u_2, __nis_pw_u_1 };
 +
        /* macros for deciding which NIS maps to use. */
 -#define       PASSWD_BYNAME(x)        ((x)->maptype == NISMAP_MASTER \
 -                                  ? "master.passwd.byname" : "passwd.byname")
 -#define       PASSWD_BYUID(x)         ((x)->maptype == NISMAP_MASTER \
 -                                  ? "master.passwd.byuid" : "passwd.byuid")
 +#define       PASSWD_BYNAME(x)        ((x)->maptype == NISMAP_MASTER ? 
__nis_pw_n_1 : __nis_pw_n_2)
 +#define       PASSWD_BYUID(x)         ((x)->maptype == NISMAP_MASTER ? 
__nis_pw_u_1 : __nis_pw_u_2)
  
  static int
  _nis_start(struct nis_state *state)
 @@ -1236,7 +1242,7 @@
   */
  static int
  _nis_pwscan(int *retval, struct passwd *pw, char *buffer, size_t buflen,
 -      struct nis_state *state, const char *map)
 +      struct nis_state *state, const char * const *map_arr)
  {
        char    *data;
        int     nisr, rv, datalen;
 @@ -1245,7 +1251,7 @@
        _DIAGASSERT(pw != NULL);
        _DIAGASSERT(buffer != NULL);
        _DIAGASSERT(state != NULL);
 -      _DIAGASSERT(map != NULL);
 +      _DIAGASSERT(map_arr != NULL);
  
        *retval = 0;
  
 @@ -1257,9 +1263,10 @@
  
        data = NULL;
        rv = NS_NOTFOUND;
 +      _DIAGASSERT(state->maptype > 0 && state->maptype < 
sizeof(map_arr)/sizeof(const char*));
  
                                                        /* search map */
 -      nisr = yp_match(state->domain, map, buffer, (int)strlen(buffer),
 +      nisr = yp_match(state->domain, map_arr[state->maptype], buffer, 
(int)strlen(buffer),
            &data, &datalen);
        switch (nisr) {
        case 0:
 @@ -1494,7 +1501,7 @@
        snprintf(_nis_passwdbuf, sizeof(_nis_passwdbuf), "%u", (unsigned 
int)uid);
        rv = _nis_pwscan(&rerror, &_nis_passwd,
            _nis_passwdbuf, sizeof(_nis_passwdbuf),
 -          &_nis_state, PASSWD_BYUID(&_nis_state));
 +          &_nis_state, __nis_pw_u_map);
        if (!_nis_state.stayopen)
                _nis_end(&_nis_state);
        if (rv == NS_SUCCESS && uid == _nis_passwd.pw_uid)
 @@ -1522,14 +1529,21 @@
        _DIAGASSERT(result != NULL);
  
        *result = NULL;
 -      memset(&state, 0, sizeof(state));
 -      rv = _nis_start(&state);
 -      if (rv != NS_SUCCESS)
 -              return rv;
        snprintf(buffer, buflen, "%u", (unsigned int)uid);
 -      rv = _nis_pwscan(retval, pw, buffer, buflen,
 -          &state, PASSWD_BYUID(&state));
 -      _nis_end(&state);
 +/* remark: we run under a global mutex inside of this module ... */
 +      if (_nis_state.stayopen)
 +        { /* use global state only if stayopen is set - otherwiese we would 
blow up getpwent_r() ... */
 +          rv = _nis_pwscan(retval, pw, buffer, buflen,
 +              &_nis_state, __nis_pw_u_map);
 +        }
 +      else
 +        { /* keep old semantic if no stayopen set - no need to call 
_nis_start() here - _nis_pwscan() will do it for us ... */
 +          /* use same way as in getgrent.c ... */
 +          memset(&state, 0, sizeof(state));
 +          rv = _nis_pwscan(retval, pw, buffer, buflen,
 +              &state, __nis_pw_u_map);
 +          _nis_end(&state);
 +        }
        if (rv != NS_SUCCESS)
                return rv;
        if (uid == pw->pw_uid) {
 @@ -1557,7 +1571,7 @@
        snprintf(_nis_passwdbuf, sizeof(_nis_passwdbuf), "%s", name);
        rv = _nis_pwscan(&rerror, &_nis_passwd,
            _nis_passwdbuf, sizeof(_nis_passwdbuf),
 -          &_nis_state, PASSWD_BYNAME(&_nis_state));
 +          &_nis_state, __nis_pw_n_map);
        if (!_nis_state.stayopen)
                _nis_end(&_nis_state);
        if (rv == NS_SUCCESS && strcmp(name, _nis_passwd.pw_name) == 0)
 @@ -1586,13 +1600,20 @@
  
        *result = NULL;
        snprintf(buffer, buflen, "%s", name);
 -      memset(&state, 0, sizeof(state));
 -      rv = _nis_start(&state);
 -      if (rv != NS_SUCCESS)
 -              return rv;
 -      rv = _nis_pwscan(retval, pw, buffer, buflen,
 -          &state, PASSWD_BYNAME(&state));
 -      _nis_end(&state);
 +/* remark: we run under a global mutex inside of this module ... */
 +      if (_nis_state.stayopen)
 +        { /* use global state only if stayopen is set - otherwiese we would 
blow up getpwent_r() ... */
 +          rv = _nis_pwscan(retval, pw, buffer, buflen,
 +              &_nis_state, __nis_pw_n_map);
 +        }
 +      else
 +        { /* keep old semantic if no stayopen set - no need to call 
_nis_start() here - _nis_pwscan() will do it for us ... */
 +          /* use same way as in getgrent.c ... */
 +          memset(&state, 0, sizeof(state));
 +          rv = _nis_pwscan(retval, pw, buffer, buflen,
 +              &state, __nis_pw_n_map);
 +          _nis_end(&state);
 +        }
        if (rv != NS_SUCCESS)
                return rv;
        if (strcmp(name, pw->pw_name) == 0) {
 @@ -2048,7 +2069,7 @@
                                state->mode = COMPAT_FULL;
                                                /* reset passwd_compat search */
  /* XXXREENTRANT: setpassent is not thread safe ? */
 -                              (void) _passwdcompat_setpassent(0);
 +                              (void) 
_passwdcompat_setpassent(_compat_state.stayopen);
                                break;
                        case '@':               /* `+@netgroup' */
                                state->mode = COMPAT_NETGROUP;
 
 
 --------------040207020604010308030301--
 


Home | Main Index | Thread Index | Old Index