On 4/24/12 11:10 AM, Bernd Schubert wrote: > On 04/24/2012 12:42 AM, Andreas Dilger wrote: >> On 2012-04-23, at 5:23 PM, Eric Sandeen wrote: >>> I'm curious about the above as well as: >>> >>> case SEEK_END: >>> if (unlikely(offset> 0)) >>> goto out_err; /* not supported for directories */ >>> >>> The previous .llseek handler, and the generic handler for other >>> filesystems, allow seeking past the end of the dir AFAICT. (not >>> sure why you'd want to, but I don't see that you'd get an error >>> back). >>> >>> Is there a reason to uniquely exclude it in ext4? Does that line up with POSIX? >> >> I don't know what the origin of this was... I don't think there is >> a real reason for it except that it doesn't make any sense to do >> so. >> > > I think I added that. According to pubs.opengroup.org: > (http://pubs.opengroup.org/onlinepubs/009695399/functions/seekdir.html) > > void seekdir(DIR *dirp, long loc); > > <quote> > > If the value of loc was not obtained from an earlier call to > telldir(), or if a call to rewinddir() occurred between the call to > telldir() and the call to seekdir(), the results of subsequent calls > to readdir() are unspecified. > > </quote> > > > As telldir(), which should correlate to 'case SEEK_CUR' will not > provide invalid values, the behaviour is undefined. > > > Also, > > > case SEEK_END: > [...] > if (dx_dir) > offset += ext4_get_htree_eof(file); > else > offset += inode->i_size; > [...] > > > if (!dx_dir) { > if (offset > inode->i_sb->s_maxbytes) > goto out_err; > } else if (offset > ext4_get_htree_eof(file)) > goto out_err; > > > > > Hence, the additional: > > case SEEK_END: > if (unlikely(offset> 0)) > goto out_err; /* not supported for directories */ > > > is just a shortcut to avoid useless calculations. > > Unless I missed something, it only remains the question if could > break existing applications relying on undefined behaviour. However, > I have no idea how an application might trigger that? (other lists removed at this point, this is ext4-specific) I know I'm being a little pedantic w/ the late review here.... It seems like the only differences between ext4_dir_llseek and the old ext4_llseek are these: 1) For SEEK_END, we now return -EINVAL for a positive offset (i.e. past EOF) 2) For SEEK_END, we seek to ext4_get_htree_eof() not to inode->i_size 3) For SEEK_SET, we impose different limits for max offset - s_maxbytes / ext4_get_htree_eof for !dx/dx, vs. s_bitmap_maxbytes/s_maxbytes Do any of these changes relate to the hash collision problem? Are any of them uniquely required for ext4, enough to warrant cut & paste of the vfs llseek code (again?) What I'm getting at is: what are the reasons that we cannot use generic_file_llseek_size(), maybe with a new argument to specify a non-standard location for SEEK_END. Such a change would require a solid explanation, but it'd probably go in if it meant one less seek implementation to worry about. -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
