PERFORCE change 123551 for review

John Baldwin jhb at freebsd.org
Mon Jul 16 12:41:42 UTC 2007


On Sunday 15 July 2007 04:11:28 pm Garrett Cooper wrote:
> http://perforce.freebsd.org/chv.cgi?CH=123551
> 
> Change 123551 by gcooper at optimus-revised_pkgtools on 2007/07/15 20:10:53
> 
> 	Bad case to exit on. A return code of 0 from read(2) good, -1 is bad..

0 means EOF.  You really want something like this:

ssize_t nread;

nread = read(fileno(fd), contents, sb.st_size);
if (nread < 0) {
	cleanup(0);
	err(2, "%s: read('%s')", __func__, fname);
} else if (nread != sb.st_size) {
	cleanup(0);
	errx(2, "%s: short read on '%s' (%zd of %ju bytes)", __func__,
	    fname, nread, (uintmax_t)sb.st_size);
}

However, why not just mmap(2) the file rather than read it?

struct stat sb;
void *p;
int fd;

fd = open(fname, O_RDONLY);
if (fd < 0)
	err(1, "open");
if (fstat(fd, &sb) < 0)
	err(1, "fstat");
p = mmap(NULL, sb.st_size, PROT_READ, MAP_SHARED, fd, 0);
if (p == MAP_FAILED)
	err(1, "mmap");

Now p points to the file's contents, and it is more efficient than read(2) 
since you don't copy the data around as often.  The only limitation is if the 
packing list file is ever going to be really big (say larger than 1GB) in 
which case you might run out of address space on 32-bit machines.  However, 
if you are already reading the entire file into a single buffer then you 
aren't worried about that.

-- 
John Baldwin


More information about the p4-projects mailing list