Module Name: src Committed By: dholland Date: Mon Aug 7 06:53:49 UTC 2017
Modified Files: src/sys/ufs/lfs: ulfs_vnops.c src/sys/ufs/ufs: ufs_vnops.c Log Message: Tidy up ufs_readdir. First step only; there's plenty more that could be done to improve this code. To generate a diff of this commit: cvs rdiff -u -r1.50 -r1.51 src/sys/ufs/lfs/ulfs_vnops.c cvs rdiff -u -r1.237 -r1.238 src/sys/ufs/ufs/ufs_vnops.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Modified files: Index: src/sys/ufs/lfs/ulfs_vnops.c diff -u src/sys/ufs/lfs/ulfs_vnops.c:1.50 src/sys/ufs/lfs/ulfs_vnops.c:1.51 --- src/sys/ufs/lfs/ulfs_vnops.c:1.50 Fri Aug 4 07:27:42 2017 +++ src/sys/ufs/lfs/ulfs_vnops.c Mon Aug 7 06:53:49 2017 @@ -1,4 +1,4 @@ -/* $NetBSD: ulfs_vnops.c,v 1.50 2017/08/04 07:27:42 maya Exp $ */ +/* $NetBSD: ulfs_vnops.c,v 1.51 2017/08/07 06:53:49 dholland Exp $ */ /* from NetBSD: ufs_vnops.c,v 1.232 2016/05/19 18:32:03 riastradh Exp */ /*- @@ -67,7 +67,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: ulfs_vnops.c,v 1.50 2017/08/04 07:27:42 maya Exp $"); +__KERNEL_RCSID(0, "$NetBSD: ulfs_vnops.c,v 1.51 2017/08/07 06:53:49 dholland Exp $"); #if defined(_KERNEL_OPT) #include "opt_lfs.h" @@ -769,129 +769,173 @@ ulfs_readdir(void *v) kauth_cred_t a_cred; int *a_eofflag; off_t **a_cookies; - int *ncookies; + int *a_ncookies; } */ *ap = v; + + /* vnode and fs */ struct vnode *vp = ap->a_vp; - LFS_DIRHEADER *cdp, *ecdp; - struct dirent *ndp; - char *cdbuf, *ndbuf, *endp; - struct uio auio, *uio; - struct iovec aiov; - int error; - size_t count, ccount, rcount, cdbufsz, ndbufsz; - off_t off, *ccp; - off_t startoff; - size_t skipbytes; struct ulfsmount *ump = VFSTOULFS(vp->v_mount); struct lfs *fs = ump->um_lfs; + /* caller's buffer */ + struct uio *calleruio = ap->a_uio; + off_t startoffset, endoffset; + size_t callerbytes; + off_t curoffset; + /* dirent production buffer */ + char *direntbuf; + size_t direntbufmax; + struct dirent *dirent, *stopdirent; + /* output cookies array */ + off_t *cookies; + size_t numcookies, maxcookies; + /* disk buffer */ + off_t physstart, physend; + size_t skipstart, dropend; + char *rawbuf; + size_t rawbufmax, rawbytes; + struct uio rawuio; + struct iovec rawiov; + LFS_DIRHEADER *rawdp, *stoprawdp; + /* general */ + int error; KASSERT(VOP_ISLOCKED(vp)); - uio = ap->a_uio; - count = uio->uio_resid; - rcount = count - ((uio->uio_offset + count) & (fs->um_dirblksiz - 1)); + /* figure out where we want to read */ + callerbytes = calleruio->uio_resid; + startoffset = calleruio->uio_offset; + endoffset = startoffset + callerbytes; - if (rcount < LFS_DIRECTSIZ(fs, 0) || count < _DIRENT_MINSIZE(ndp)) + if (callerbytes < _DIRENT_MINSIZE(dirent)) { + /* no room for even one struct dirent */ return EINVAL; + } - startoff = uio->uio_offset & ~(fs->um_dirblksiz - 1); - skipbytes = uio->uio_offset - startoff; - rcount += skipbytes; - - auio.uio_iov = &aiov; - auio.uio_iovcnt = 1; - auio.uio_offset = startoff; - auio.uio_resid = rcount; - UIO_SETUP_SYSSPACE(&auio); - auio.uio_rw = UIO_READ; - cdbufsz = rcount; - cdbuf = kmem_alloc(cdbufsz, KM_SLEEP); - aiov.iov_base = cdbuf; - aiov.iov_len = rcount; - error = VOP_READ(vp, &auio, 0, ap->a_cred); + /* round start and end down to block boundaries */ + physstart = startoffset & ~(off_t)(fs->um_dirblksiz - 1); + physend = endoffset & ~(off_t)(fs->um_dirblksiz - 1); + skipstart = startoffset - physstart; + dropend = endoffset - physend; + + if (callerbytes - dropend < LFS_DIRECTSIZ(fs, 0)) { + /* no room for even one dirheader + name */ + return EINVAL; + } + + /* how much to actually read */ + rawbufmax = callerbytes + skipstart - dropend; + + /* read it */ + rawbuf = kmem_alloc(rawbufmax, KM_SLEEP); + rawiov.iov_base = rawbuf; + rawiov.iov_len = rawbufmax; + rawuio.uio_iov = &rawiov; + rawuio.uio_iovcnt = 1; + rawuio.uio_offset = physstart; + rawuio.uio_resid = rawbufmax; + UIO_SETUP_SYSSPACE(&rawuio); + rawuio.uio_rw = UIO_READ; + error = VOP_READ(vp, &rawuio, 0, ap->a_cred); if (error != 0) { - kmem_free(cdbuf, cdbufsz); + kmem_free(rawbuf, rawbufmax); return error; } + rawbytes = rawbufmax - rawuio.uio_resid; - rcount -= auio.uio_resid; + /* the raw entries to iterate over */ + rawdp = (LFS_DIRHEADER *)(void *)rawbuf; + stoprawdp = (LFS_DIRHEADER *)(void *)&rawbuf[rawbytes]; - cdp = (LFS_DIRHEADER *)(void *)cdbuf; - ecdp = (LFS_DIRHEADER *)(void *)&cdbuf[rcount]; + /* allocate space to produce dirents into */ + direntbufmax = callerbytes; + direntbuf = kmem_alloc(direntbufmax, KM_SLEEP); - ndbufsz = count; - ndbuf = kmem_alloc(ndbufsz, KM_SLEEP); - ndp = (struct dirent *)(void *)ndbuf; - endp = &ndbuf[count]; + /* the dirents to iterate over */ + dirent = (struct dirent *)(void *)direntbuf; + stopdirent = (struct dirent *)(void *)&direntbuf[direntbufmax]; - off = uio->uio_offset; + /* the output "cookies" (seek positions of directory entries) */ if (ap->a_cookies) { - ccount = rcount / LFS_DIRECTSIZ(fs, 1); - ccp = *(ap->a_cookies) = malloc(ccount * sizeof(*ccp), + numcookies = 0; + maxcookies = rawbytes / LFS_DIRECTSIZ(fs, 1); + cookies = malloc(maxcookies * sizeof(*cookies), M_TEMP, M_WAITOK); } else { /* XXX: GCC */ - ccount = 0; - ccp = NULL; + maxcookies = 0; + cookies = NULL; } - while (cdp < ecdp) { - if (skipbytes > 0) { - if (lfs_dir_getreclen(fs, cdp) <= skipbytes) { - skipbytes -= lfs_dir_getreclen(fs, cdp); - cdp = LFS_NEXTDIR(fs, cdp); + /* now produce the dirents */ + curoffset = calleruio->uio_offset; + while (rawdp < stoprawdp) { + if (skipstart > 0) { + /* drain skipstart */ + if (lfs_dir_getreclen(fs, rawdp) <= skipstart) { + skipstart -= lfs_dir_getreclen(fs, rawdp); + rawdp = LFS_NEXTDIR(fs, rawdp); continue; } - /* - * invalid cookie. - */ + /* caller's start position wasn't on an entry */ error = EINVAL; goto out; } - if (lfs_dir_getreclen(fs, cdp) == 0) { - struct dirent *ondp = ndp; - ndp->d_reclen = _DIRENT_MINSIZE(ndp); - ndp = _DIRENT_NEXT(ndp); - ondp->d_reclen = 0; - cdp = ecdp; + if (lfs_dir_getreclen(fs, rawdp) == 0) { + struct dirent *save = dirent; + dirent->d_reclen = _DIRENT_MINSIZE(dirent); + dirent = _DIRENT_NEXT(dirent); + save->d_reclen = 0; + rawdp = stoprawdp; break; } - ndp->d_type = lfs_dir_gettype(fs, cdp); - ndp->d_namlen = lfs_dir_getnamlen(fs, cdp); - ndp->d_reclen = _DIRENT_RECLEN(ndp, ndp->d_namlen); - if ((char *)(void *)ndp + ndp->d_reclen + - _DIRENT_MINSIZE(ndp) > endp) + + /* copy the header */ + dirent->d_type = lfs_dir_gettype(fs, rawdp); + dirent->d_namlen = lfs_dir_getnamlen(fs, rawdp); + dirent->d_reclen = _DIRENT_RECLEN(dirent, dirent->d_namlen); + + /* stop if there isn't room for the name AND another header */ + if ((char *)(void *)dirent + dirent->d_reclen + + _DIRENT_MINSIZE(dirent) > (char *)(void *)stopdirent) break; - ndp->d_fileno = lfs_dir_getino(fs, cdp); - (void)memcpy(ndp->d_name, lfs_dir_nameptr(fs, cdp), - ndp->d_namlen); - memset(&ndp->d_name[ndp->d_namlen], 0, - ndp->d_reclen - _DIRENT_NAMEOFF(ndp) - ndp->d_namlen); - off += lfs_dir_getreclen(fs, cdp); + + /* copy the name (and inode (XXX: why after the test?)) */ + dirent->d_fileno = lfs_dir_getino(fs, rawdp); + (void)memcpy(dirent->d_name, lfs_dir_nameptr(fs, rawdp), + dirent->d_namlen); + memset(&dirent->d_name[dirent->d_namlen], 0, + dirent->d_reclen - _DIRENT_NAMEOFF(dirent) + - dirent->d_namlen); + + /* onward */ + curoffset += lfs_dir_getreclen(fs, rawdp); if (ap->a_cookies) { - KASSERT(ccp - *(ap->a_cookies) < ccount); - *(ccp++) = off; + KASSERT(numcookies < maxcookies); + cookies[numcookies++] = curoffset; } - ndp = _DIRENT_NEXT(ndp); - cdp = LFS_NEXTDIR(fs, cdp); + dirent = _DIRENT_NEXT(dirent); + rawdp = LFS_NEXTDIR(fs, rawdp); } - count = ((char *)(void *)ndp - ndbuf); - error = uiomove(ndbuf, count, uio); + /* transfer the dirents to the caller's buffer */ + callerbytes = ((char *)(void *)dirent - direntbuf); + error = uiomove(direntbuf, callerbytes, calleruio); + out: + calleruio->uio_offset = curoffset; if (ap->a_cookies) { if (error) { - free(*(ap->a_cookies), M_TEMP); - *(ap->a_cookies) = NULL; - *(ap->a_ncookies) = 0; + free(cookies, M_TEMP); + *ap->a_cookies = NULL; + *ap->a_ncookies = 0; } else { - *ap->a_ncookies = ccp - *(ap->a_cookies); + *ap->a_cookies = cookies; + *ap->a_ncookies = numcookies; } } - uio->uio_offset = off; - kmem_free(ndbuf, ndbufsz); - kmem_free(cdbuf, cdbufsz); - *ap->a_eofflag = VTOI(vp)->i_size <= uio->uio_offset; + kmem_free(direntbuf, direntbufmax); + kmem_free(rawbuf, rawbufmax); + *ap->a_eofflag = VTOI(vp)->i_size <= calleruio->uio_offset; return error; } Index: src/sys/ufs/ufs/ufs_vnops.c diff -u src/sys/ufs/ufs/ufs_vnops.c:1.237 src/sys/ufs/ufs/ufs_vnops.c:1.238 --- src/sys/ufs/ufs/ufs_vnops.c:1.237 Wed Apr 26 03:02:49 2017 +++ src/sys/ufs/ufs/ufs_vnops.c Mon Aug 7 06:53:48 2017 @@ -1,4 +1,4 @@ -/* $NetBSD: ufs_vnops.c,v 1.237 2017/04/26 03:02:49 riastradh Exp $ */ +/* $NetBSD: ufs_vnops.c,v 1.238 2017/08/07 06:53:48 dholland Exp $ */ /*- * Copyright (c) 2008 The NetBSD Foundation, Inc. @@ -66,7 +66,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: ufs_vnops.c,v 1.237 2017/04/26 03:02:49 riastradh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: ufs_vnops.c,v 1.238 2017/08/07 06:53:48 dholland Exp $"); #if defined(_KERNEL_OPT) #include "opt_ffs.h" @@ -1219,19 +1219,11 @@ ufs_readdir(void *v) kauth_cred_t a_cred; int *a_eofflag; off_t **a_cookies; - int *ncookies; + int *a_ncookies; } */ *ap = v; + + /* vnode and fs */ struct vnode *vp = ap->a_vp; - struct direct *cdp, *ecdp; - struct dirent *ndp; - char *cdbuf, *ndbuf, *endp; - struct uio auio, *uio; - struct iovec aiov; - int error; - size_t count, ccount, rcount, cdbufsz, ndbufsz; - off_t off, *ccp; - off_t startoff; - size_t skipbytes; struct ufsmount *ump = VFSTOUFS(vp->v_mount); int nswap = UFS_MPNEEDSWAP(ump); #if BYTE_ORDER == LITTLE_ENDIAN @@ -1239,116 +1231,171 @@ ufs_readdir(void *v) #else int needswap = ump->um_maxsymlinklen <= 0 && nswap != 0; #endif - uio = ap->a_uio; - count = uio->uio_resid; - rcount = count - ((uio->uio_offset + count) & (ump->um_dirblksiz - 1)); + /* caller's buffer */ + struct uio *calleruio = ap->a_uio; + off_t startoffset, endoffset; + size_t callerbytes; + off_t curoffset; + /* dirent production buffer */ + char *direntbuf; + size_t direntbufmax; + struct dirent *dirent, *stopdirent; + /* output cookies array */ + off_t *cookies; + size_t numcookies, maxcookies; + /* disk buffer */ + off_t physstart, physend; + size_t skipstart, dropend; + char *rawbuf; + size_t rawbufmax, rawbytes; + struct uio rawuio; + struct iovec rawiov; + struct direct *rawdp, *stoprawdp; + /* general */ + int error; + + KASSERT(VOP_ISLOCKED(vp)); - if (rcount < _DIRENT_MINSIZE(cdp) || count < _DIRENT_MINSIZE(ndp)) + /* figure out where we want to read */ + callerbytes = calleruio->uio_resid; + startoffset = calleruio->uio_offset; + endoffset = startoffset + callerbytes; + + if (callerbytes < _DIRENT_MINSIZE(dirent)) { + /* no room for even one struct dirent */ return EINVAL; + } - startoff = uio->uio_offset & ~(ump->um_dirblksiz - 1); - skipbytes = uio->uio_offset - startoff; - rcount += skipbytes; - - auio.uio_iov = &aiov; - auio.uio_iovcnt = 1; - auio.uio_offset = startoff; - auio.uio_resid = rcount; - UIO_SETUP_SYSSPACE(&auio); - auio.uio_rw = UIO_READ; - cdbufsz = rcount; - cdbuf = kmem_alloc(cdbufsz, KM_SLEEP); - aiov.iov_base = cdbuf; - aiov.iov_len = rcount; - error = UFS_BUFRD(vp, &auio, 0, ap->a_cred); + /* round start and end down to block boundaries */ + physstart = startoffset & ~(off_t)(ump->um_dirblksiz - 1); + physend = endoffset & ~(off_t)(ump->um_dirblksiz - 1); + skipstart = startoffset - physstart; + dropend = endoffset - physend; + + if (callerbytes - dropend < _DIRENT_MINSIZE(rawdp)) { + /* no room for even one struct direct */ + return EINVAL; + } + + /* how much to actually read */ + rawbufmax = callerbytes + skipstart - dropend; + + /* read it */ + rawbuf = kmem_alloc(rawbufmax, KM_SLEEP); + rawiov.iov_base = rawbuf; + rawiov.iov_len = rawbufmax; + rawuio.uio_iov = &rawiov; + rawuio.uio_iovcnt = 1; + rawuio.uio_offset = physstart; + rawuio.uio_resid = rawbufmax; + UIO_SETUP_SYSSPACE(&rawuio); + rawuio.uio_rw = UIO_READ; + error = UFS_BUFRD(vp, &rawuio, 0, ap->a_cred); if (error != 0) { - kmem_free(cdbuf, cdbufsz); + kmem_free(rawbuf, rawbufmax); return error; } + rawbytes = rawbufmax - rawuio.uio_resid; - rcount -= auio.uio_resid; + /* the raw entries to iterate over */ + rawdp = (struct direct *)(void *)rawbuf; + stoprawdp = (struct direct *)(void *)&rawbuf[rawbytes]; - cdp = (struct direct *)(void *)cdbuf; - ecdp = (struct direct *)(void *)&cdbuf[rcount]; + /* allocate space to produce dirents into */ + direntbufmax = callerbytes; + direntbuf = kmem_alloc(direntbufmax, KM_SLEEP); - ndbufsz = count; - ndbuf = kmem_alloc(ndbufsz, KM_SLEEP); - ndp = (struct dirent *)(void *)ndbuf; - endp = &ndbuf[count]; + /* the dirents to iterate over */ + dirent = (struct dirent *)(void *)direntbuf; + stopdirent = (struct dirent *)(void *)&direntbuf[direntbufmax]; - off = uio->uio_offset; + /* the output "cookies" (seek positions of directory entries) */ if (ap->a_cookies) { - ccount = rcount / _DIRENT_RECLEN(cdp, 1); - ccp = *(ap->a_cookies) = malloc(ccount * sizeof(*ccp), + numcookies = 0; + maxcookies = rawbytes / _DIRENT_RECLEN(rawdp, 1); + cookies = malloc(maxcookies * sizeof(*cookies), M_TEMP, M_WAITOK); } else { /* XXX: GCC */ - ccount = 0; - ccp = NULL; + maxcookies = 0; + cookies = NULL; } - while (cdp < ecdp) { - cdp->d_reclen = ufs_rw16(cdp->d_reclen, nswap); - if (skipbytes > 0) { - if (cdp->d_reclen <= skipbytes) { - skipbytes -= cdp->d_reclen; - cdp = _DIRENT_NEXT(cdp); + /* now produce the dirents */ + curoffset = calleruio->uio_offset; + while (rawdp < stoprawdp) { + rawdp->d_reclen = ufs_rw16(rawdp->d_reclen, nswap); + if (skipstart > 0) { + /* drain skipstart */ + if (rawdp->d_reclen <= skipstart) { + skipstart -= rawdp->d_reclen; + rawdp = _DIRENT_NEXT(rawdp); continue; } - /* - * invalid cookie. - */ + /* caller's start position wasn't on an entry */ error = EINVAL; goto out; } - if (cdp->d_reclen == 0) { - struct dirent *ondp = ndp; - ndp->d_reclen = _DIRENT_MINSIZE(ndp); - ndp = _DIRENT_NEXT(ndp); - ondp->d_reclen = 0; - cdp = ecdp; + if (rawdp->d_reclen == 0) { + struct dirent *save = dirent; + dirent->d_reclen = _DIRENT_MINSIZE(dirent); + dirent = _DIRENT_NEXT(dirent); + save->d_reclen = 0; + rawdp = stoprawdp; break; } + + /* copy the header */ if (needswap) { - ndp->d_type = cdp->d_namlen; - ndp->d_namlen = cdp->d_type; + dirent->d_type = rawdp->d_namlen; + dirent->d_namlen = rawdp->d_type; } else { - ndp->d_type = cdp->d_type; - ndp->d_namlen = cdp->d_namlen; + dirent->d_type = rawdp->d_type; + dirent->d_namlen = rawdp->d_namlen; } - ndp->d_reclen = _DIRENT_RECLEN(ndp, ndp->d_namlen); - if ((char *)(void *)ndp + ndp->d_reclen + - _DIRENT_MINSIZE(ndp) > endp) + dirent->d_reclen = _DIRENT_RECLEN(dirent, dirent->d_namlen); + + /* stop if there isn't room for the name AND another header */ + if ((char *)(void *)dirent + dirent->d_reclen + + _DIRENT_MINSIZE(dirent) > (char *)(void *)stopdirent) break; - ndp->d_fileno = ufs_rw32(cdp->d_ino, nswap); - (void)memcpy(ndp->d_name, cdp->d_name, ndp->d_namlen); - memset(&ndp->d_name[ndp->d_namlen], 0, - ndp->d_reclen - _DIRENT_NAMEOFF(ndp) - ndp->d_namlen); - off += cdp->d_reclen; + + /* copy the name (and inode (XXX: why after the test?)) */ + dirent->d_fileno = ufs_rw32(rawdp->d_ino, nswap); + (void)memcpy(dirent->d_name, rawdp->d_name, dirent->d_namlen); + memset(&dirent->d_name[dirent->d_namlen], 0, + dirent->d_reclen - _DIRENT_NAMEOFF(dirent) + - dirent->d_namlen); + + /* onward */ + curoffset += rawdp->d_reclen; if (ap->a_cookies) { - KASSERT(ccp - *(ap->a_cookies) < ccount); - *(ccp++) = off; + KASSERT(numcookies < maxcookies); + cookies[numcookies++] = curoffset; } - ndp = _DIRENT_NEXT(ndp); - cdp = _DIRENT_NEXT(cdp); + dirent = _DIRENT_NEXT(dirent); + rawdp = _DIRENT_NEXT(rawdp); } - count = ((char *)(void *)ndp - ndbuf); - error = uiomove(ndbuf, count, uio); + /* transfer the dirents to the caller's buffer */ + callerbytes = ((char *)(void *)dirent - direntbuf); + error = uiomove(direntbuf, callerbytes, calleruio); + out: + calleruio->uio_offset = curoffset; if (ap->a_cookies) { if (error) { - free(*(ap->a_cookies), M_TEMP); - *(ap->a_cookies) = NULL; - *(ap->a_ncookies) = 0; + free(cookies, M_TEMP); + *ap->a_cookies = NULL; + *ap->a_ncookies = 0; } else { - *ap->a_ncookies = ccp - *(ap->a_cookies); + *ap->a_cookies = cookies; + *ap->a_ncookies = numcookies; } } - uio->uio_offset = off; - kmem_free(ndbuf, ndbufsz); - kmem_free(cdbuf, cdbufsz); - *ap->a_eofflag = VTOI(vp)->i_size <= uio->uio_offset; + kmem_free(direntbuf, direntbufmax); + kmem_free(rawbuf, rawbufmax); + *ap->a_eofflag = VTOI(vp)->i_size <= calleruio->uio_offset; return error; }