Re: Two patches to slurp.cxx and MapsBuilder.cxx

[Phil Muldoon <pmuldoon at redhat dot com>]

Andrew Cagney wrote:
> Phil,
>
> this looks exactly how one would expect, and your code's level of 
> return-checcking paranoia exceeds all expectations :-)
>
> However, if it is nits you want :-)
>
> - a few cases of << throw exception ; return NULL >> - compiler should 
> have warned about this :-/

Oddly enough, the compiler did not warn, or I did not see it. Is the 
warning level on the C blobs cranked up as high as the ECJ warnings? 
Anyway, will do fix.
> - mention why only BUFSIZ-1 is read - leave space for the trailing NUL 
> - just 'cos I spent 1/2 thinking why is that being done

Righto.
> I'm trying to think how to test this and nothing comes to mind 'cos 
> trying to mimic the wack-o kernel behavior is near impossible.

I spent an hour thinking on this yesterday. One of the reason I checked 
it in as uslurp instead of replacing slurp is I was pondering an 
adequate test case for it.  I'm still pondering ;)

Regards

Phil
> A
> > Index: frysk-sys/frysk/sys/proc/cni/slurp.cxx
> > ===================================================================
> > RCS file: /cvs/frysk/frysk-sys/frysk/sys/proc/cni/slurp.cxx,v
> > retrieving revision 1.5
> > retrieving revision 1.6
> > diff -u -r1.5 -r1.6
> > --- frysk-sys/frysk/sys/proc/cni/slurp.cxx    6 Sep 2006 21:55:09 
> > -0000    1.5
> > +++ frysk-sys/frysk/sys/proc/cni/slurp.cxx    16 Oct 2006 21:19:58 
> > -0000    1.6
> > @@ -50,6 +50,72 @@
> >  #include "frysk/sys/cni/Errno.hxx"
> >  #include "frysk/sys/proc/cni/slurp.hxx"
> >  
> > +jbyteArray
> > +uslurp(int pid, const char* name) +{
> > +  // Get the file name.
> > +  char file[FILENAME_MAX];
> > +  if (::snprintf (file, sizeof file, "/proc/%d/%s", (int) pid, name)
> > +      >= FILENAME_MAX)
> > +    throwRuntimeException ("snprintf: buffer overflow");
> > +
> > +   // This implementation unfortunatly double buffers the data.
> > +  int len = 0;
> > +  long current_offset = 0;
> > +   +  // Malloc one page size for first read
> > +  char * buf = (char*)::malloc(sizeof(char)*BUFSIZ);
> > +
> > +  // Check malloc ok
> > +  if (buf == NULL) {
> > +    throwRuntimeException ("cannot malloc initial slurp buffer");
> > +    return NULL;
> > +  }
> > +
> > +  // Open the file file.
> > +  errno = 0;
> > +  int fd = ::open (file, O_RDONLY);
> > +  if (errno != 0) {
> > +     ::free(buf);
> > +     return NULL;
> > +  }
> > +
> > +  do  {
> > +    // Reads upto BUFSIZ bytes from /proc. It appears reading maps 
> > and possibly
> > +    // other files from /proc you can only read the file at 4096 max 
> > bytes per time. +    // Hence the read * n, realloc *n dance. Ref SW 
> > #3370
> > +    errno = 0;
> > +    len = ::read (fd, buf+current_offset, (BUFSIZ-1));
> > +    if (errno != 0) {
> > +      ::close (fd);
> > +      ::free(buf);
> > +      return NULL;
> > +    }
> > +    current_offset+=len;
> > +
> > +    // Don't trust realloc with my pointer, I want to free it if 
> > something goes +    // wrong.
> > +    char *tmp = (char*)::realloc(buf, sizeof(char) * (current_offset 
> > + BUFSIZ));
> > +    if (tmp == NULL) {
> > +      ::close(fd);
> > +      ::free(buf);
> > +      throwRuntimeException ("slurp realloc failed");
> > +      return NULL;
> > +    } else
> > +       buf = tmp;
> > +  } while (len > 0);
> > +
> > +  ::close(fd);
> > +
> > +  // Null terminate the buffer.
> > +  buf[current_offset] = '\0';
> > +
> > +  jbyteArray jbuf = JvNewByteArray (current_offset);
> > +  ::memcpy (elements (jbuf), buf, current_offset);
> > +  ::free(buf);
> > +  +  return jbuf;
> > +}
> >  
> >  int
> >  slurp (int pid, const char* name, char buf[], long sizeof_buf)
> >   
> > ------------------------------------------------------------------------
> >
> > Index: frysk-sys/frysk/sys/proc/cni/MapsBuilder.cxx
> > ===================================================================
> > RCS file: /cvs/frysk/frysk-sys/frysk/sys/proc/cni/MapsBuilder.cxx,v
> > retrieving revision 1.2
> > retrieving revision 1.3
> > diff -u -r1.2 -r1.3
> > --- frysk-sys/frysk/sys/proc/cni/MapsBuilder.cxx    3 Nov 2005 
> > 02:08:21 -0000    1.2
> > +++ frysk-sys/frysk/sys/proc/cni/MapsBuilder.cxx    16 Oct 2006 
> > 21:19:58 -0000    1.3
> > @@ -51,7 +51,7 @@
> >  jboolean
> >  frysk::sys::proc::MapsBuilder::construct (jint pid)
> >  {
> > -  jbyteArray buf = slurp (pid, "maps");
> > +  jbyteArray buf = uslurp (pid, "maps");
> >    if (buf == NULL)
> >      return false;
> >    buildBuffer (buf);
> > @@ -95,7 +95,7 @@
> >        while (isblank (*p))
> >      p++;
> >        jint pathnameOffset = p - start;
> > -      while (*p != '\0' && !isspace (*p)) {
> > +      while (*p != '\0' && *p != '\n') {
> >      p++;
> >        }
> >        int pathnameLength = p - start - pathnameOffset;
> >