[PATCH] xfstests: add new getdents test
Alex Elder
aelder at sgi.com
Fri Sep 23 08:56:22 CDT 2011
On Fri, 2011-09-23 at 16:03 +0300, Grazvydas Ignotas wrote:
> On Thu, Sep 22, 2011 at 11:18 PM, Alex Elder <aelder at sgi.com> wrote:
> > On Mon, 2011-09-12 at 03:19 +0300, Grazvydas Ignotas wrote:
> >> The test checks if no duplicate d_off values are returned and that
> >> those values are seekable to the right inodes.
> >>
> >> Signed-off-by: Grazvydas Ignotas <notasas at gmail.com>
> >
> > I have two minor comments on the C program below,
> > but even if you don't want to address them this
> > looks good.
> >
> > Reviewed-by: Alex Elder <aelder at sgi.com>
> >
. . .
> >> +static uint64_t d_off_histoty[HISTORY_LEN];
> >> +static uint64_t d_ino_histoty[HISTORY_LEN];
> >
> > Is "histoty" intentional or a typo?
>
> whoops, it's a typo. I might send a patch for this later.
I will change this for you before committing.
> >
> >> +int
> >> +main(int argc, char *argv[])
. . .
> >> + nread = syscall(SYS_getdents64, fd, buf, BUF_SIZE);
> >
> > You could just use sizeof (struct linux_dirent_64) rather than
> > BUF_SIZE here. I suppose it doesn't hurt but there's no real
> > sense in reading more than the one you're going to look at.
>
> I'm not sure if reading partial entry is allowed, manpage says it may
> fail with EINVAL if buffer size is too small..
I will keep this as-is. I was wrong about the size you should
pass, but my point still stands... For a single entry you need
to pass a buffer big enough to hold a linux_dirent64 structure
*plus* the maximum-sized name that entry could contain. That
appears to be 256 bytes, though POSIX allows more.
Anyway, if you or anyone else wants to try to change this
in the future to read in a smaller amount here, that's fine
but it's not necessary now.
Bottom line, I'll make the change I mentioned above and
will commit the result.
-Alex
More information about the xfs
mailing list