Re: [2.4.19] read(2) and page aligned buffers

[Clayton Weaver]

(It's not an alignment issue. Still broken read()ing whole file
into page-aligned malloc() buffers and MAP_PRIVATE|MAP_ANONYMOUS mmap()
buffers, while not broken mmap()ing the file directly.)

Ok, bizarrely enough (or not), I couldn't reproduce it with a
test program that isolates the functions that return error
in my program in a program of their own that just reads a
dir, open()s files, malloc()s space, read()s the file into the space,
free()s the buffer, and close()s the file. I even incorporated
the sha1 checksum functions from textutils that I use in the actual
program seeing the error into the test program and ran the buffers
through them, still without reproducing the error. I used one of the dirs
where I actually see the errors with malloc() + read() that I originally
reported.

But the test program isn't significantly different from the would-be
production code that sees the error. I've posted it below in case
it is informative.

(preliminaries: the file size is ok, it's an lstat() st_size that
 mmap(...MAP_SHARED...) has no issues with. The open() in the
 caller of gethash() is merely 

     fd = open(fdata->name, O_RDONLY);
     if (fd >= 0) {

followed by an fcntl() read lock, etc. Eventually it gets down to

     if (gethash(fd, fdata->size, fdata->name,
		 REC_CHKSUM(*current)) == NULL) {
       return -1;
     }

Here is gethash(), the version that sees the error from read():

/*****
 * gethash():
 *
 * args: const int fd            open file descriptor for file to checksum
 *       const off_t len         length of file
 *       const char * pathname   for error reporting
 *       void * outsum           where to put checksum
 *
 * returns:  void * (outsum) or NULL
 *
 * Notes: Takes a file and size as args, and returns the address of
 *        outsum with the checksum in the first sizeof(checksum)
 *        bytes (outsum can point to the tail of an f_rec or to some
 *        other buffer with sufficient space). sizeof(checksum)
 *        is 20 bytes for sha1 checksums.
 *
 *        Calls sha1 code lifted from gnu sha.c in gnu textutils-2.0.21.
 *****/

/* assume that the appropriate #includes are up here */

void * gethash(const int, const off_t, const char *, void *);

/* extern prototypes; see headers */

void * gethash(const int fd, const off_t len, const char * pathname,
	       void * outsum)
{
  const char * funcname = "gethash()";
  struct sha_ctx bchk_ctx;
  void * filebuf;

#ifndef NDEBUG
  if (!len || !pathname || !outsum) {
    bchk_err(funcname, nullarg);
    return NULL;
  }
#endif

  sha_init_ctx(&bchk_ctx);

  filebuf = xmalloc((size_t) len);
  if (!filebuf) {
    return NULL;
  }

  if (wrap_read(fd, filebuf, (size_t) len) != (ssize_t) len) {
    rpt_syserr(funcname, read_err, pathname, NULL);
    return NULL;
  }

  /* (from the version that works fine)
  filebuf = wrap_mmap((size_t) len, PROT_READ, (int) fd);
  if (filebuf == NULL) {
    rpt_syserr(funcname, mmap_err, pathname, NULL);
    return NULL;
  }
  */

  /* sha_process_bytes() does its own % 64 check and calls
     sha_process_block() internally to process chunks that are
     a multiple of 64 bytes before handling any remainder < 64 */

  sha_process_bytes(filebuf, (size_t) len, &bchk_ctx);
  (void) sha_finish_ctx(&bchk_ctx, outsum);
  free(filebuf);
  /*  (void) munmap(filebuf, (size_t) len); */

  return outsum;
}

Here is wrap_read() (assume that the appropriate #includes are there):

/* read() wrapper with incremental retry on interrupt */

ssize_t wrap_read(int fd, void * buf, size_t count)
{
  ssize_t retval;
  ssize_t tmpret;

#ifndef NDEBUG
  const char * funcname = "wrap_read()";
  if (!buf || !count || fd < 0) {
    bchk_err(funcname, nullarg);
    return -1;
  }
#endif

  retval = 0;
  errno = 0;
  do {
    tmpret = read(fd, ((char *) buf + retval), count - retval);
    if (tmpret + retval == (ssize_t) count) {
      return (ssize_t) count;
    }
    else {
      switch (tmpret) {
      case -1:
	switch(errno) {
	case EINTR:
	case EAGAIN:
	  break;

	default:  /* real error */
	  return retval;
	  break;
	}
	break;

      case 0:
	return retval;
	break;

      default: /* partial read */
	switch(errno) {
	case EINTR:    /* interrupted by signal */
	case EAGAIN:   /* O_NONBLOCK ? */
	  retval += tmpret;
	  break;

	default:  /* real error */
	  return (tmpret + retval);
	  break;
	}
	break;
      }
    }
  } while (retval < count);

  return retval;
}

And that's bloody it.

?

(I thought, "stack", but what's downstream? 2 sha1 calls that
don't seem to do anything evil in the test program that attempted
to isolate the error and free(). Doesn't add up.)

Regards,

Clayton Weaver
<mailto: [e-mail]>

-- 
_______________________________________________
Sign-up for your own FREE Personalized E-mail at Mail.com
http://www.mail.com/?sr=signup

Single & ready to mingle? lavalife.com:  Where singles click. Free to Search!
http://www.lavalife.com/mailcom.epl?a=2116