Re: No timeout for nss_ldap?

On Thu, 2008-01-03 at 14:56 +0100, Tony Earnshaw wrote:
> I'm now trying gradually to put nss-ldapd in place instead of Red
> Hat's $DEITY-awful nss_ldap, but it seems it has a bug with large
> Posix groups at the moment (our largest has about 900+ memberUids).

This is a known problem with nss-ldapd 0.5 and is fixed (partially) in
svn. The attached patch against 0.5 should solve this issue and is
tested with groups with up to 1000 members.

--
-- arthur - arthur@... - http://ch.tudelft.nl/~arthur --

[nss-ldapd-large-groups.patch]

---

diff -Naur nss-ldapd-0.5.orig/common/tio.c nss-ldapd-0.5/common/tio.c
--- nss-ldapd-0.5.orig/common/tio.c 2007-10-27 23:38:37.000000000 +0200
+++ nss-ldapd-0.5/common/tio.c 2008-01-03 17:15:55.000000000 +0100
@@ -2,7 +2,7 @@
    tio.c - timed io functions
    This file is part of the nss-ldapd library.
 
-   Copyright (C) 2007 Arthur de Jong
+   Copyright (C) 2007, 2008 Arthur de Jong
 
    This library is free software; you can redistribute it and/or
    modify it under the terms of the GNU Lesser General Public
@@ -60,6 +60,7 @@
   struct tio_buffer *writebuffer;
   struct timeval readtimeout;
   struct timeval writetimeout;
+  int read_resettable; /* whether the tio_reset() function can be called */
 #ifdef DEBUG_TIO_STATS
   /* this is used to collect statistics on the use of the streams
      and can be used to tune the buffer sizes */
@@ -156,6 +157,7 @@
   fp->readtimeout.tv_usec=readtimeout->tv_usec;
   fp->writetimeout.tv_sec=writetimeout->tv_sec;
   fp->writetimeout.tv_usec=writetimeout->tv_usec;
+  fp->read_resettable=0;
 #ifdef DEBUG_TIO_STATS
   fp->byteswritten=0;
   fp->bytesread=0;
@@ -221,10 +223,9 @@
   /* loop until we have returned all the needed data */
   while (1)
   {
-    /* return any data already in the buffer */
+    /* check if we have enough data in the buffer */
     if (fp->readbuffer->len >= count)
     {
-      /* we have enough data in our buffer */
       if (count>0)
       {
         if (ptr!=NULL)
@@ -235,26 +236,32 @@
       }
       return 0;
     }
-    else
+    /* empty what we have and continue from there */
+    if (fp->readbuffer->len > 0)
     {
-      /* empty what we have and continue from there */
-      if (fp->readbuffer->len > 0)
+      if (ptr!=NULL)
       {
-        if (ptr!=NULL)
-          memcpy(ptr,fp->readbuffer->buffer+fp->readbuffer->start,fp->readbuffer->len);
-        count-=fp->readbuffer->len;
-        if (ptr!=NULL)
-          ptr+=fp->readbuffer->len;
+        memcpy(ptr,fp->readbuffer->buffer+fp->readbuffer->start,fp->readbuffer->len);
+        ptr+=fp->readbuffer->len;
       }
-      /* flag the buffer as empty */
+      count-=fp->readbuffer->len;
+    }
+    /* if we have room in the buffer for more don't clear the buffer */
+    if ((fp->read_resettable)&&((fp->readbuffer->start+fp->readbuffer->len)<TIO_BUFFERSIZE))
+    {
+      fp->readbuffer->start+=fp->readbuffer->len;
+    }
+    else
+    {
       fp->readbuffer->start=0;
-      fp->readbuffer->len=0;
+      fp->read_resettable=0;
     }
+    fp->readbuffer->len=0;
     /* wait until we have input */
     if (tio_select(fp->fd,1,&deadline))
       return -1;
     /* read the input in the buffer */
-    rv=read(fp->fd,fp->readbuffer->buffer,TIO_BUFFERSIZE);
+    rv=read(fp->fd,fp->readbuffer->buffer+fp->readbuffer->start,TIO_BUFFERSIZE-fp->readbuffer->start);
     /* check for errors */
     if ((rv==0)||((rv<0)&&(errno!=EINTR)&&(errno!=EAGAIN)))
       return -1; /* something went wrong with the read */
@@ -386,3 +393,33 @@
   /* return the result of the earlier operations */
   return retv;
 }
+
+void tio_mark(TFILE *fp)
+{
+  /* ensure that we have a read buffer */
+  if (fp->readbuffer==NULL)
+  {
+    fp->readbuffer=tio_buffer_new();
+    if (fp->readbuffer==NULL)
+      return; /* error allocating buffer */
+  }
+  /* move any data in the buffer to the start of the buffer */
+  if ((fp->readbuffer->start>0)&&(fp->readbuffer->len>0))
+  {
+    memmove(fp->readbuffer->buffer,fp->readbuffer->buffer+fp->readbuffer->start,fp->readbuffer->len);
+    fp->readbuffer->start=0;
+  }
+  /* mark the stream as resettable */
+  fp->read_resettable=1;
+}
+
+int tio_reset(TFILE *fp)
+{
+  /* check if the stream is (still) resettable */
+  if ((!fp->read_resettable)||(fp->readbuffer==NULL))
+    return -1;
+  /* reset the buffer */
+  fp->readbuffer->len+=fp->readbuffer->start;
+  fp->readbuffer->start=0;
+  return 0;
+}
diff -Naur nss-ldapd-0.5.orig/common/tio.h nss-ldapd-0.5/common/tio.h
--- nss-ldapd-0.5.orig/common/tio.h 2007-07-23 23:15:19.000000000 +0200
+++ nss-ldapd-0.5/common/tio.h 2008-01-03 17:15:55.000000000 +0100
@@ -2,7 +2,7 @@
    tio.h - timed io functions
    This file is part of the nss-ldapd library.
 
-   Copyright (C) 2007 Arthur de Jong
+   Copyright (C) 2007, 2008 Arthur de Jong
 
    This library is free software; you can redistribute it and/or
    modify it under the terms of the GNU Lesser General Public
@@ -22,9 +22,14 @@
 
 /*
 
-   Add some documentation here.
+   TODO: Add some documentation here.
 
-   the SIGPIPE signal should be ignored
+   the SIGPIPE signal should be ignored (is ignored in this code)
+
+   This library is not thread safe. You cannot share TFILE objects between
+   threads and expect to be able to read and write from them in different
+   threads. All the state is in the TFILE object so calls to this library on
+   different objects can be done in parallel.
 
 */
 
@@ -36,29 +41,39 @@
 
 #include "compat/attrs.h"
 
-/* generic file handle used for reading and writing
-   (something like FILE from stdio.h) */
+/* This is a generic file handle used for reading and writing
+   (something like FILE from stdio.h). */
 typedef struct tio_fileinfo TFILE;
 
-/* Open a new TFILE based on the file descriptor.
-   The timeout is set for any operation.
-   the timeout value is copied so may be dereferenced after the call. */
+/* Open a new TFILE based on the file descriptor. The timeout is set for any
+   operation. The timeout value is copied so may be dereferenced after the
+   call. */
 TFILE *tio_fdopen(int fd,struct timeval *readtimeout,struct timeval *writetimeout)
   LIKE_MALLOC MUST_USE;
 
 /* Read the specified number of bytes from the stream. */
-int tio_read(TFILE *fp, void *buf, size_t count);
+int tio_read(TFILE *fp,void *buf,size_t count);
 
 /* Read and discard the specified number of bytes from the stream. */
-int tio_skip(TFILE *fp, size_t count);
+int tio_skip(TFILE *fp,size_t count);
 
 /* Write the specified buffer to the stream. */
-int tio_write(TFILE *fp, const void *buf, size_t count);
+int tio_write(TFILE *fp,const void *buf,size_t count);
 
 /* Write out all buffered data to the stream. */
 int tio_flush(TFILE *fp);
 
-/* this also closes the underlying file descriptor */
+/* Flush the streams and closes the underlying file descriptor. */
 int tio_close(TFILE *fp);
 
+/* Store the current position in the stream so that we can jump back to it
+   with the tio_reset() function. */
+void tio_mark(TFILE *fp);
+
+/* Rewinds the stream to the point set by tio_mark(). Note that this only
+   resets the read stream and not the write stream. This function returns
+   whether the reset was successful (this function may fail if the buffers
+   were full). */
+int tio_reset(TFILE *fp);
+
 #endif /* _TIO_H */
diff -Naur nss-ldapd-0.5.orig/nss/common.h nss-ldapd-0.5/nss/common.h
--- nss-ldapd-0.5.orig/nss/common.h 2007-07-23 23:15:19.000000000 +0200
+++ nss-ldapd-0.5/nss/common.h 2008-01-03 17:15:55.000000000 +0100
@@ -2,7 +2,7 @@
    common.h - common functions for NSS lookups
 
    Copyright (C) 2006 West Consulting
-   Copyright (C) 2006, 2007 Arthur de Jong
+   Copyright (C) 2006, 2007, 2008 Arthur de Jong
 
    This library is free software; you can redistribute it and/or
    modify it under the terms of the GNU Lesser General Public
@@ -94,8 +94,6 @@
    Something more inteligent (e.g. ungetting the read data from
    the stream) should be implemented. */
 #define ERROR_OUT_BUFERROR(fp) \
-  (void)tio_close(fp); \
-  fp=NULL; \
   *errnop=ERANGE; \
   return NSS_STATUS_TRYAGAIN;
 
@@ -125,14 +123,14 @@
    the result structure, the user buffer with length and the
    errno to return. This macro should be called through some of
    the customized ones below. */
-#define NSS_BYGEN(action,param,readfn) \
+#define NSS_BYGEN(action,writefn,readfn) \
   TFILE *fp; \
   int32_t tmpint32; \
   enum nss_status retv; \
   /* open socket and write request */ \
   OPEN_SOCK(fp); \
   WRITE_REQUEST(fp,action); \
-  param; \
+  writefn; \
   WRITE_FLUSH(fp); \
   /* read response header */ \
   READ_RESPONSEHEADER(fp,action); \
@@ -140,7 +138,7 @@
   READ_RESPONSE_CODE(fp); \
   retv=readfn; \
   /* close socket and we're done */ \
-  if (retv==NSS_STATUS_SUCCESS) \
+  if ((retv==NSS_STATUS_SUCCESS)||(retv==NSS_STATUS_TRYAGAIN)) \
     (void)tio_close(fp); \
   return retv;
 
@@ -189,11 +187,22 @@
     *errnop=ENOENT; \
     return NSS_STATUS_UNAVAIL; \
   } \
+  /* prepare for buffer errors */ \
+  tio_mark(fp); \
   /* read a response */ \
   READ_RESPONSE_CODE(fp); \
   retv=readfn; \
   /* check read result */ \
-  if (retv!=NSS_STATUS_SUCCESS) \
+  if (retv==NSS_STATUS_TRYAGAIN) \
+  { \
+    /* if we have a full buffer try to reset the stream */ \
+    if (tio_reset(fp)) \
+    { \
+      tio_close(fp); \
+      fp=NULL; \
+    } \
+  } \
+  else if (retv!=NSS_STATUS_SUCCESS) \
     fp=NULL; /* file should be closed by now */ \
   return retv;
 
diff -Naur nss-ldapd-0.5.orig/tests/test_tio.c nss-ldapd-0.5/tests/test_tio.c
--- nss-ldapd-0.5.orig/tests/test_tio.c 2007-08-19 16:20:40.000000000 +0200
+++ nss-ldapd-0.5/tests/test_tio.c 2008-01-03 17:15:55.000000000 +0100
@@ -2,7 +2,7 @@
    test_tio.c - simple test for the tio module
    This file is part of the nss-ldapd library.
 
-   Copyright (C) 2007 Arthur de Jong
+   Copyright (C) 2007, 2008 Arthur de Jong
 
    This library is free software; you can redistribute it and/or
    modify it under the terms of the GNU Lesser General Public
@@ -55,7 +55,7 @@
   timeout.tv_sec=hargs->timeout;
   timeout.tv_usec=0;
   /* open the file */
-  fp = tio_fdopen(hargs->fd,&timeout,&timeout);
+  fp=tio_fdopen(hargs->fd,&timeout,&timeout);
   assert(fp!=NULL);
   /* write the blocks */
   i=0;
@@ -87,7 +87,7 @@
   timeout.tv_sec=hargs->timeout;
   timeout.tv_usec=0;
   /* open the file */
-  fp = tio_fdopen(hargs->fd,&timeout,&timeout);
+  fp=tio_fdopen(hargs->fd,&timeout,&timeout);
   assert(fp!=NULL);
   /* read the blocks */
   i=0;
@@ -115,7 +115,7 @@
   buf=(uint8_t *)malloc(hargs->blocksize);
   assert(buf!=NULL);
   /* open the file */
-  fp = fdopen(hargs->fd,"wb");
+  fp=fdopen(hargs->fd,"wb");
   assert(fp!=NULL);
   /* write the blocks */
   i=0;
@@ -139,7 +139,7 @@
   size_t i,j,k;
   struct helper_args *hargs=(struct helper_args *)arg;
   /* open the file */
-  fp = fdopen(hargs->fd,"rb");
+  fp=fdopen(hargs->fd,"rb");
   assert(fp!=NULL);
   /* read the blocks */
   i=0;
@@ -189,6 +189,73 @@
   return 0;
 }
 
+static void test_reset(void)
+{
+  int sp[2];
+  pthread_t wthread;
+  struct helper_args wargs;
+  TFILE *fp;
+  struct timeval timeout;
+  size_t i,j,k,save;
+  uint8_t buf[10];
+  /* set up the socket pair */
+  assert(socketpair(AF_UNIX,SOCK_STREAM,0,sp)==0);
+  /* start the writer thread */
+  wargs.fd=sp[0];
+  wargs.blocksize=4*1024; /* the current TIO_BUFFERSIZE */
+  wargs.blocks=5;
+  wargs.timeout=2;
+  assert(pthread_create(&wthread,NULL,help_tiowriter,&wargs)==0);
+  /* set up read handle */
+  timeout.tv_sec=2;
+  timeout.tv_usec=0;
+  fp=tio_fdopen(sp[1],&timeout,&timeout);
+  assert(fp!=NULL);
+  /* perform 20 reads */
+  i=0;
+  for (k=0;k<20;k++)
+  {
+    assert(tio_read(fp,buf,sizeof(buf))==0);
+    /* check the buffer */
+    for (j=0;j<sizeof(buf);j++)
+      assert(buf[j]==(uint8_t)(i++));
+  }
+  /* mark and perform another 2 reads */
+  tio_mark(fp);
+  save=i;
+  for (k=0;k<2;k++)
+  {
+    assert(tio_read(fp,buf,sizeof(buf))==0);
+    /* check the buffer */
+    for (j=0;j<sizeof(buf);j++)
+      assert(buf[j]==(uint8_t)(i++));
+  }
+  /* reset and perform the same 2 reads again and 500 more */
+  assert(tio_reset(fp)==0);
+  i=save;
+  for (k=0;k<502;k++)
+  {
+    assert(tio_read(fp,buf,sizeof(buf))==0);
+    /* check the buffer */
+    for (j=0;j<sizeof(buf);j++)
+      assert(buf[j]==(uint8_t)(i++));
+  }
+  /* check that reset is no longer possible */
+  assert(tio_reset(fp)!=0);
+  /* read the remainder of the data 1526 reads */
+  for (k=0;k<1526;k++)
+  {
+    assert(tio_read(fp,buf,sizeof(buf))==0);
+    /* check the buffer */
+    for (j=0;j<sizeof(buf);j++)
+      assert(buf[j]==(uint8_t)(i++));
+  }
+  /* close the file */
+  assert(tio_close(fp)==0);
+  /* wait for the writer thread to die */
+  assert(pthread_join(wthread,NULL)==0);
+}
+
 /* the main program... */
 int main(int UNUSED(argc),char UNUSED(*argv[]))
 {
@@ -202,5 +269,7 @@
   /* writer closes file sooner */
 /*  test_blocks(4*1023,20,20*1023,5); */
 /*  test_blocks(10,9,10,10); */
+  /* set tio_mark() and tio_reset() functions */
+  test_reset();
   return 0;
 }

---