Date: Sat, 25 Apr 2015 08:54:23 -0400 (EDT) From: Rick Macklem <rmacklem@uoguelph.ca> To: Jilles Tjoelker <jilles@stack.nl> Cc: Julian Elischer <julian@freebsd.org>, freebsd-current@freebsd.org, John Baldwin <jhb@freebsd.org> Subject: Re: readdir/telldir/seekdir problem (i think) Message-ID: <768602615.25669083.1429966463806.JavaMail.root@uoguelph.ca> In-Reply-To: <20150424215249.GA96554@stack.nl>
next in thread | previous in thread | raw e-mail | index | archive | help
Jilles Toelker wrote: > On Fri, Apr 24, 2015 at 04:28:12PM -0400, John Baldwin wrote: > > Yes, this isn't at all safe. There's no guarantee whatsoever that > > the offset on the directory fd that isn't something returned by > > getdirentries has any meaning. In particular, the size of the > > directory entry in a random filesystem might be a different size > > than the structure returned by getdirentries (since it converts > > things into a FS-independent format). > > > This might work for UFS by accident, but this is probably why ZFS > > doesn't work. > > > However, this might be properly fixed by the thing that ino64 is > > doing where each directory entry returned by getdirentries gives > > you a seek offset that you _can_ directly seek to (as opposed to > > seeking to the start of the block and then walking forward N > > entries until you get an inter-block entry that is the same). > > The ino64 branch only reserves space for d_off and does not use it in > any way. This is appropriate since actually using d_off is a major > feature addition. > Just fyi, I did a patch: http://people.freebsd.org/~rmacklem/64bitfileno.patch which does fill this field in for the various file systems and copies the new "struct dirent" to "struct dirent32", so it works with an unchanged userland. Since the ino64 folks felt this should come after the ino64 change, I didn't pursue committing it. To commit it to head, it would have to be modified slightly, so that the new structure is called "struct dirent64", so it doesn't screw up userland builds using "struct dirent", but I think that is the only change required for it to go into head/current if people felt that was appropriate. (Oh, and it called d_off "d_cookie", although I have no strong preference w.r.t. what it is called.) If you look at it, you'll see most of the changes are for NFS, but the rest were pretty trivial. rick > A proper d_off would still be useful even if UFS's readdir keeps > masking > off the offset so a directory read always starts at the beginning of > a > 512-byte directory block, since this allows more distinct offset > values > than safely using getdirentries()'s *basep. With d_off, one outer > loop > must read at least one directory block to avoid spinning > indefinitely, > while using getdirentries()'s *basep requires reading the whole > getdirentries() buffer. > > Some Linux filesystems go further and provide a unique d_off for each > entry. > > Another idea would be to store the last d_ino instead of dd_loc into > the > struct ddloc. On seekdir(), this would seek to loc_seek as before and > skip entries until that d_ino is found, or to the start of the buffer > if > not found (and possibly return some entries again that should not be > returned, but Samba copes with that). > > -- > Jilles Tjoelker > _______________________________________________ > freebsd-current@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-current > To unsubscribe, send any mail to > "freebsd-current-unsubscribe@freebsd.org" >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?768602615.25669083.1429966463806.JavaMail.root>