Message ID | 20140327211516.GB2476@domone.podge |
---|---|
State | New |
Headers | show |
On Thu, Mar 27, 2014 at 10:15:16PM +0100, Ondřej Bílka wrote: > Had typo there, here is fixed patch. Please add a note on how you tested the fix. > * nscd/netgroupcache.c: Clean up implementation. Incorrect ChangeLog entry. > diff --git a/nscd/netgroupcache.c b/nscd/netgroupcache.c > index 820d823..1412113 100644 > --- a/nscd/netgroupcache.c > +++ b/nscd/netgroupcache.c > @@ -138,8 +138,10 @@ addgetnetgrentX (struct database_dyn *db, int fd, request_header *req, > char *key_copy = NULL; > struct __netgrent data; > size_t buflen = MAX (1024, sizeof (*dataset) + req->key_len); > + size_t tempbuflen = 1024; > size_t buffilled = sizeof (*dataset); > char *buffer = NULL; > + char *tempbuf; > size_t nentries = 0; > size_t group_len = strlen (key) + 1; > union > @@ -159,6 +161,8 @@ addgetnetgrentX (struct database_dyn *db, int fd, request_header *req, > > memset (&data, '\0', sizeof (data)); > buffer = xmalloc (buflen); > + tempbuf = xmalloc (tempbuflen); > + > first_needed.elem.next = &first_needed.elem; > memcpy (first_needed.elem.name, key, group_len); > data.needed_groups = &first_needed.elem; > @@ -201,8 +205,7 @@ addgetnetgrentX (struct database_dyn *db, int fd, request_header *req, > while (1) > { > int e; > - status = getfct.f (&data, buffer + buffilled, > - buflen - buffilled - req->key_len, &e); > + status = getfct.f (&data, tempbuf, tempbuflen, &e); > if (status == NSS_STATUS_RETURN > || status == NSS_STATUS_NOTFOUND) > /* This was either the last one for this group or the > @@ -220,53 +223,11 @@ addgetnetgrentX (struct database_dyn *db, int fd, request_header *req, > size_t userlen = strlen (nuser ?: "") + 1; > size_t domainlen = strlen (ndomain ?: "") + 1; > > - if (nhost == NULL || nuser == NULL || ndomain == NULL > - || nhost > nuser || nuser > ndomain) > + size_t newlen = buffilled + hostlen + userlen + domainlen; > + if (newlen + req->key_len > buflen) Might as well put req->key_len in newlen and simplify this further. > { > - const char *last = nhost; > - if (last == NULL > - || (nuser != NULL && nuser > last)) > - last = nuser; > - if (last == NULL > - || (ndomain != NULL && ndomain > last)) > - last = ndomain; > - > - size_t bufused > - = (last == NULL > - ? buffilled > - : last + strlen (last) + 1 - buffer); > - > - /* We have to make temporary copies. */ > - size_t needed = hostlen + userlen + domainlen; > - > - if (buflen - req->key_len - bufused < needed) > - { > - buflen += MAX (buflen, 2 * needed); > - /* Save offset in the old buffer. We don't > - bother with the NULL check here since > - we'll do that later anyway. */ > - size_t nhostdiff = nhost - buffer; > - size_t nuserdiff = nuser - buffer; > - size_t ndomaindiff = ndomain - buffer; > - > - char *newbuf = xrealloc (buffer, buflen); > - /* Fix up the triplet pointers into the new > - buffer. */ > - nhost = (nhost ? newbuf + nhostdiff > - : NULL); > - nuser = (nuser ? newbuf + nuserdiff > - : NULL); > - ndomain = (ndomain ? newbuf + ndomaindiff > - : NULL); > - buffer = newbuf; > - } > - > - nhost = memcpy (buffer + bufused, > - nhost ?: "", hostlen); > - nuser = memcpy ((char *) nhost + hostlen, > - nuser ?: "", userlen); > - ndomain = memcpy ((char *) nuser + userlen, > - ndomain ?: "", domainlen); > + buflen = 2 * newlen + req->key_len; > + buffer = xrealloc (buffer, buflen); > } > > char *wp = buffer + buffilled; > @@ -328,8 +289,8 @@ addgetnetgrentX (struct database_dyn *db, int fd, request_header *req, > } > else if (status == NSS_STATUS_UNAVAIL && e == ERANGE) > { > - buflen *= 2; > - buffer = xrealloc (buffer, buflen); > + tempbuflen *= 2; > + tempbuf = xrealloc (tempbuf, tempbuflen); This ought to be unlikely now, since the consolidated lengths of user, host and domain should fit in the initially allocated 1024 bytes. > } > } > > @@ -474,6 +435,7 @@ addgetnetgrentX (struct database_dyn *db, int fd, request_header *req, > } > > out: > + free (tempbuf); > free (buffer); > > *resultp = dataset; > Siddhesh
On Fri, Mar 28, 2014 at 08:17:37AM +0530, Siddhesh Poyarekar wrote: > On Thu, Mar 27, 2014 at 10:15:16PM +0100, Ondřej Bílka wrote: > > Had typo there, here is fixed patch. > > Please add a note on how you tested the fix. > make check on x86-64, no new failures.
On Fri, Apr 11, 2014 at 04:30:53PM +0200, Ondřej Bílka wrote: > On Fri, Mar 28, 2014 at 08:17:37AM +0530, Siddhesh Poyarekar wrote: > > On Thu, Mar 27, 2014 at 10:15:16PM +0100, Ondřej Bílka wrote: > > > Had typo there, here is fixed patch. > > > > Please add a note on how you tested the fix. > > > make check on x86-64, no new failures. make check doesn't test nscd. You'll need to test this manually by adding various types of netgroups entries in /etc/netgroup. Siddhesh
On Fri, Apr 11, 2014 at 10:24:15PM +0530, Siddhesh Poyarekar wrote: > On Fri, Apr 11, 2014 at 04:30:53PM +0200, Ondřej Bílka wrote: > > On Fri, Mar 28, 2014 at 08:17:37AM +0530, Siddhesh Poyarekar wrote: > > > On Thu, Mar 27, 2014 at 10:15:16PM +0100, Ondřej Bílka wrote: > > > > Had typo there, here is fixed patch. > > > > > > Please add a note on how you tested the fix. > > > > > make check on x86-64, no new failures. > > make check doesn't test nscd. You'll need to test this manually by > adding various types of netgroups entries in /etc/netgroup. > How, I get back to this and now study manpages which is bit confusing. I tried to install nis for that but ypcat does not seem to work, do you have a test script?
On Tue, May 27, 2014 at 10:27:09PM +0200, Ondřej Bílka wrote: > > make check doesn't test nscd. You'll need to test this manually by > > adding various types of netgroups entries in /etc/netgroup. > > > How, I get back to this and now study manpages which is bit confusing. I > tried to install nis for that but ypcat does not seem to work, do you > have a test script? You don't need NIS to test it because you can add netgroup entries in a file called /etc/netgroup. I don't have a script handy to create these, but the netgroup manpage[1] should help you with the netgroup file format to come up with one. The file format is quite simple actually. Each netgroup entry is of type: netgroup_name (host1, user1, domain1) (host2, user2, domain2) ... where the stuff in brackets is called a triplet, which specifies a combination of hostname, username and domain name that may be allowed in a netgroup. A netgroup entry can be split into multiple line with a trailing backslash (\). One may have wildcards in a triplet like so: (host1, , domain1) This means any user on host1 and domain1. You can have similar wildcards for hosts and domains as well, which makes all of the following as valid triplets: (, user, domain) (, , domain) (host, , domain) (host, , ) (host, user, ) and so on. I hope this helps. Siddhesh [1] http://linux.die.net/man/5/netgroup
On Wed, May 28, 2014 at 07:21:22AM +0530, Siddhesh Poyarekar wrote: > On Tue, May 27, 2014 at 10:27:09PM +0200, Ondřej Bílka wrote: > > > make check doesn't test nscd. You'll need to test this manually by > > > adding various types of netgroups entries in /etc/netgroup. > > > > > How, I get back to this and now study manpages which is bit confusing. I > > tried to install nis for that but ypcat does not seem to work, do you > > have a test script? > > You don't need NIS to test it because you can add netgroup entries in > a file called /etc/netgroup. I don't have a script handy to create > these, but the netgroup manpage[1] should help you with the netgroup > file format to come up with one. The file format is quite simple > actually. Each netgroup entry is of type: > > netgroup_name (host1, user1, domain1) (host2, user2, domain2) ... > I already have these but do not know how to do query these, a nscd -g gives yes cache is enabled no cache is persistent no cache is shared 211 suggested size 216064 total data pool size 0 used data pool size 28800 seconds time to live for positive entries 20 seconds time to live for negative entries 0 cache hits on positive entries 0 cache hits on negative entries
On Wed, May 28, 2014 at 11:05:21AM +0200, Ondřej Bílka wrote:
> I already have these but do not know how to do query these, a nscd -g gives
Ahh OK, the stats are broken. You just need to make sure that the
output you get with and without nscd is consistent.
Siddhesh
On Wed, May 28, 2014 at 04:43:18PM +0530, Siddhesh Poyarekar wrote: > On Wed, May 28, 2014 at 11:05:21AM +0200, Ondřej Bílka wrote: > > I already have these but do not know how to do query these, a nscd -g gives > > Ahh OK, the stats are broken. You just need to make sure that the > output you get with and without nscd is consistent. > What output?
On Wed, May 28, 2014 at 02:20:53PM +0200, Ondřej Bílka wrote: > On Wed, May 28, 2014 at 04:43:18PM +0530, Siddhesh Poyarekar wrote: > > On Wed, May 28, 2014 at 11:05:21AM +0200, Ondřej Bílka wrote: > > > I already have these but do not know how to do query these, a nscd -g gives > > > > Ahh OK, the stats are broken. You just need to make sure that the > > output you get with and without nscd is consistent. > > > What output? getent netgroup <netgroup-name>
On Wed, May 28, 2014 at 06:01:00PM +0530, Siddhesh Poyarekar wrote: > On Wed, May 28, 2014 at 02:20:53PM +0200, Ondřej Bílka wrote: > > On Wed, May 28, 2014 at 04:43:18PM +0530, Siddhesh Poyarekar wrote: > > > On Wed, May 28, 2014 at 11:05:21AM +0200, Ondřej Bílka wrote: > > > > I already have these but do not know how to do query these, a nscd -g gives > > > > > > Ahh OK, the stats are broken. You just need to make sure that the > > > output you get with and without nscd is consistent. > > > > > What output? > > getent netgroup <netgroup-name> Now it caches negative responses but getent netgroup group does not write anything. /etc/netgroup that I used is following: group (who,are,you) powerusers (,miquels,) (,torvalds,) (,fubar,) ourhosts (enterprise,,) (laforge,,) (Q,,) Is that configuration problem?
On Wed, May 28, 2014 at 08:32:16PM +0200, Ondřej Bílka wrote: > Now it caches negative responses but getent netgroup group does not > write anything. /etc/netgroup that I used is following: > > group (who,are,you) > powerusers (,miquels,) (,torvalds,) (,fubar,) > ourhosts (enterprise,,) (laforge,,) (Q,,) getent not writing anything to stdout implies that the netgroup was not found. I assume you're querying with the netgroup name: getent netgroup powerusers which should give: powerusers (,miquels,) (,torvalds,) (,fubar,) Siddhesh
On Thu, May 29, 2014 at 07:28:11AM +0530, Siddhesh Poyarekar wrote: > On Wed, May 28, 2014 at 08:32:16PM +0200, Ondřej Bílka wrote: > > Now it caches negative responses but getent netgroup group does not > > write anything. /etc/netgroup that I used is following: > > > > group (who,are,you) > > powerusers (,miquels,) (,torvalds,) (,fubar,) > > ourhosts (enterprise,,) (laforge,,) (Q,,) > > getent not writing anything to stdout implies that the netgroup was > not found. I assume you're querying with the netgroup name: > > getent netgroup powerusers > > which should give: > > powerusers (,miquels,) (,torvalds,) (,fubar,) > Then how to configure it? It always gives not found for me.
On Thu, Jun 05, 2014 at 02:00:29PM +0200, Ondřej Bílka wrote:
> Then how to configure it? It always gives not found for me.
/etc/nsswitch.conf needs to also have 'files' as a backend.
Siddhesh
On Thu, Jun 05, 2014 at 05:38:08PM +0530, Siddhesh Poyarekar wrote: > On Thu, Jun 05, 2014 at 02:00:29PM +0200, Ondřej Bílka wrote: > > Then how to configure it? It always gives not found for me. > > /etc/nsswitch.conf needs to also have 'files' as a backend. > Already have that, does not work.
diff --git a/nscd/netgroupcache.c b/nscd/netgroupcache.c index 820d823..1412113 100644 --- a/nscd/netgroupcache.c +++ b/nscd/netgroupcache.c @@ -138,8 +138,10 @@ addgetnetgrentX (struct database_dyn *db, int fd, request_header *req, char *key_copy = NULL; struct __netgrent data; size_t buflen = MAX (1024, sizeof (*dataset) + req->key_len); + size_t tempbuflen = 1024; size_t buffilled = sizeof (*dataset); char *buffer = NULL; + char *tempbuf; size_t nentries = 0; size_t group_len = strlen (key) + 1; union @@ -159,6 +161,8 @@ addgetnetgrentX (struct database_dyn *db, int fd, request_header *req, memset (&data, '\0', sizeof (data)); buffer = xmalloc (buflen); + tempbuf = xmalloc (tempbuflen); + first_needed.elem.next = &first_needed.elem; memcpy (first_needed.elem.name, key, group_len); data.needed_groups = &first_needed.elem; @@ -201,8 +205,7 @@ addgetnetgrentX (struct database_dyn *db, int fd, request_header *req, while (1) { int e; - status = getfct.f (&data, buffer + buffilled, - buflen - buffilled - req->key_len, &e); + status = getfct.f (&data, tempbuf, tempbuflen, &e); if (status == NSS_STATUS_RETURN || status == NSS_STATUS_NOTFOUND) /* This was either the last one for this group or the @@ -220,53 +223,11 @@ addgetnetgrentX (struct database_dyn *db, int fd, request_header *req, size_t userlen = strlen (nuser ?: "") + 1; size_t domainlen = strlen (ndomain ?: "") + 1; - if (nhost == NULL || nuser == NULL || ndomain == NULL - || nhost > nuser || nuser > ndomain) + size_t newlen = buffilled + hostlen + userlen + domainlen; + if (newlen + req->key_len > buflen) { - const char *last = nhost; - if (last == NULL - || (nuser != NULL && nuser > last)) - last = nuser; - if (last == NULL - || (ndomain != NULL && ndomain > last)) - last = ndomain; - - size_t bufused - = (last == NULL - ? buffilled - : last + strlen (last) + 1 - buffer); - - /* We have to make temporary copies. */ - size_t needed = hostlen + userlen + domainlen; - - if (buflen - req->key_len - bufused < needed) - { - buflen += MAX (buflen, 2 * needed); - /* Save offset in the old buffer. We don't - bother with the NULL check here since - we'll do that later anyway. */ - size_t nhostdiff = nhost - buffer; - size_t nuserdiff = nuser - buffer; - size_t ndomaindiff = ndomain - buffer; - - char *newbuf = xrealloc (buffer, buflen); - /* Fix up the triplet pointers into the new - buffer. */ - nhost = (nhost ? newbuf + nhostdiff - : NULL); - nuser = (nuser ? newbuf + nuserdiff - : NULL); - ndomain = (ndomain ? newbuf + ndomaindiff - : NULL); - buffer = newbuf; - } - - nhost = memcpy (buffer + bufused, - nhost ?: "", hostlen); - nuser = memcpy ((char *) nhost + hostlen, - nuser ?: "", userlen); - ndomain = memcpy ((char *) nuser + userlen, - ndomain ?: "", domainlen); + buflen = 2 * newlen + req->key_len; + buffer = xrealloc (buffer, buflen); } char *wp = buffer + buffilled; @@ -328,8 +289,8 @@ addgetnetgrentX (struct database_dyn *db, int fd, request_header *req, } else if (status == NSS_STATUS_UNAVAIL && e == ERANGE) { - buflen *= 2; - buffer = xrealloc (buffer, buflen); + tempbuflen *= 2; + tempbuf = xrealloc (tempbuf, tempbuflen); } } @@ -474,6 +435,7 @@ addgetnetgrentX (struct database_dyn *db, int fd, request_header *req, } out: + free (tempbuf); free (buffer); *resultp = dataset;