Subject: Re: Second patch for lseek() sparse file extension
To: None <reinoud@netbsd.org>
From: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
List: tech-kern
Date: 09/22/2006 11:33:17
> - switch (SCARG(uap, whence)) {
> - case SEEK_CUR:
> - newoff = fp->f_offset + SCARG(uap, offset);
> - break;
> - case SEEK_END:
> - error = VOP_GETATTR(vp, &vattr, cred, l);
> - if (error)
> - goto out;
> - newoff = SCARG(uap, offset) + vattr.va_size;
> - break;
> - case SEEK_SET:
> - newoff = SCARG(uap, offset);
> - break;
> - default:
> - error = EINVAL;
> - goto out;
> - }
> - if ((error = VOP_SEEK(vp, fp->f_offset, newoff, cred)) != 0)
> + error = VOP_GETATTR(vp, &vattr, cred, l);
> + if (error)
> goto out;
>
> - *(off_t *)retval = fp->f_offset = newoff;
> - out:
> + error = VOP_SEEK(vp, fp->f_offset, SCARG(uap, whence),
> + SCARG(uap, offset), &newoffset, &vattr, cred);
passing vattr seems a bad idea to me.
VOP_SEEK (ie. filesystem) itself should have a better idea of
attributes than callers.
> #
> -# Needs work: Is newoff right? What's it mean?
> # XXX Locking protocol?
> +# XXX kauth_cred_t cred is not used
> #
> vop_seek {
> IN struct vnode *vp;
> - IN off_t oldoff;
> - IN off_t newoff;
> + IN off_t oldoffset;
> + IN int whence;
> + IN off_t givenoffset;
> + OUT off_t *newoffset;
> + IN struct vattr *vattr;
> IN kauth_cred_t cred;
> };
you need to introduce some locking because you are
introducing a race with block allocation.
YAMAMOTO Takashi