[PATCH] xfstests: add new getdents test

[PATCH] xfstests: add new getdents test

[Grazvydas Ignotas]

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@gmail.com>
---
 257                 |   56 +++++++++++++++++++++++
 257.out             |    2 +
 group               |    1 +
 src/Makefile        |    2 +-
 src/t_dir_offset2.c |  124 +++++++++++++++++++++++++++++++++++++++++++=
++++++++
 5 files changed, 184 insertions(+), 1 deletions(-)
 create mode 100755 257
 create mode 100644 257.out
 create mode 100644 src/t_dir_offset2.c

diff --git a/257 b/257
new file mode 100755
index 0000000..fb81e07
--- /dev/null
+++ b/257
@@ -0,0 +1,56 @@
+#! /bin/bash
+#
+# Check that no duplicate d_off values are returned and that those
+# values are seekable. Most work is done by the C program here.
+#
+#---------------------------------------------------------------------=
--
+# Copyright (c) 2011 Gra=C5=BEvydas Ignotas
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#---------------------------------------------------------------------=
--
+#
+# creator
+owner=3Dnotasas@gmail.com
+
+seq=3D`basename $0`
+echo "QA output created by $seq"
+
+here=3D`pwd`
+tmp=3D/tmp/$$
+status=3D1	# failure is the default!
+
+_cleanup()
+{
+	rm -rf $TEST_DIR/ttt
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+# real QA test starts here
+_supported_fs generic
+_supported_os Linux
+
+mkdir $TEST_DIR/ttt
+for n in {1..168}; do
+    touch $TEST_DIR/ttt/$n;
+done
+src/t_dir_offset2 $TEST_DIR/ttt
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=3D0
diff --git a/257.out b/257.out
new file mode 100644
index 0000000..ac232ec
--- /dev/null
+++ b/257.out
@@ -0,0 +1,2 @@
+QA output created by 257
+*** done
diff --git a/group b/group
index 0c746c8..a742495 100644
--- a/group
+++ b/group
@@ -370,3 +370,4 @@ deprecated
 254 auto quick
 255 auto quick prealloc
 256 auto quick
+257 dir auto quick
diff --git a/src/Makefile b/src/Makefile
index 91088bf..67250ee 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -17,7 +17,7 @@ LINUX_TARGETS =3D xfsctl bstat t_mtab getdevicesize p=
reallo_rw_pattern_reader \
 	preallo_rw_pattern_writer ftrunc trunc fs_perms testx looptest \
 	locktest unwritten_mmap bulkstat_unlink_test t_stripealign \
 	bulkstat_unlink_test_modified t_dir_offset t_futimens t_immutable \
-	stale_handle pwrite_mmap_blocked fstrim
+	stale_handle pwrite_mmap_blocked fstrim t_dir_offset2
=20
 SUBDIRS =3D
=20
diff --git a/src/t_dir_offset2.c b/src/t_dir_offset2.c
new file mode 100644
index 0000000..1375c3b
--- /dev/null
+++ b/src/t_dir_offset2.c
@@ -0,0 +1,124 @@
+/*
+ * Copyright (c) 2011 Gra=C5=BEvydas Ignotas
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software Foundation=
,
+ * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+/*
+ * This test checks if no duplicate d_off values are returned and
+ * that these offsets are seekable to entry with the right inode.
+ */
+
+#include <fcntl.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <stdint.h>
+#include <stdlib.h>
+#include <sys/stat.h>
+#include <sys/syscall.h>
+
+struct linux_dirent64 {
+	uint64_t	d_ino;
+	uint64_t	d_off;
+	unsigned short	d_reclen;
+	unsigned char	d_type;
+	char		d_name[0];
+};
+
+#define BUF_SIZE 4096
+#define HISTORY_LEN 1024
+
+static uint64_t d_off_histoty[HISTORY_LEN];
+static uint64_t d_ino_histoty[HISTORY_LEN];
+
+int
+main(int argc, char *argv[])
+{
+	int fd, nread;
+	char buf[BUF_SIZE];
+	struct linux_dirent64 *d;
+	int bpos, total, i;
+	off_t lret;
+	int retval =3D EXIT_SUCCESS;
+
+	fd =3D open(argv[1], O_RDONLY | O_DIRECTORY);
+	if (fd < 0) {
+		perror("open");
+		exit(EXIT_FAILURE);
+	}
+
+	total =3D 0;
+	for ( ; ; ) {
+		nread =3D syscall(SYS_getdents64, fd, buf, BUF_SIZE);
+		if (nread =3D=3D -1) {
+			perror("getdents");
+			exit(EXIT_FAILURE);
+		}
+
+		if (nread =3D=3D 0)
+			break;
+
+		for (bpos =3D 0; bpos < nread; total++) {
+			d =3D (struct linux_dirent64 *) (buf + bpos);
+
+			if (total >=3D HISTORY_LEN) {
+				fprintf(stderr, "too many files\n");
+				break;
+			}
+
+			for (i =3D 0; i < total; i++)
+			{
+				if (d_off_histoty[i] =3D=3D d->d_off) {
+					fprintf(stderr, "entries %d and %d have duplicate d_off %lld\n",
+						i, total, (long long int)d->d_off);
+					retval =3D EXIT_FAILURE;
+				}
+			}
+			d_off_histoty[total] =3D d->d_off;
+			d_ino_histoty[total] =3D d->d_ino;
+			bpos +=3D d->d_reclen;
+		}
+	}
+
+	/* check if seek works correctly */
+	d =3D (struct linux_dirent64 *)buf;
+	for (i =3D total - 1; i >=3D 0; i--)
+	{
+		lret =3D lseek(fd, i > 0 ? d_off_histoty[i - 1] : 0, SEEK_SET);
+		if (lret =3D=3D -1) {
+			perror("lseek");
+			exit(EXIT_FAILURE);
+		}
+
+		nread =3D syscall(SYS_getdents64, fd, buf, BUF_SIZE);
+		if (nread =3D=3D -1) {
+			perror("getdents");
+			exit(EXIT_FAILURE);
+		}
+
+		if (nread =3D=3D 0) {
+			fprintf(stderr, "getdents returned 0 on entry %d\n", i);
+			retval =3D EXIT_FAILURE;
+		}
+
+		if (d->d_ino !=3D d_ino_histoty[i]) {
+			fprintf(stderr, "entry %d has inode %lld, expected %lld\n",
+				i, (long long int)d->d_ino, (long long int)d_ino_histoty[i]);
+			retval =3D EXIT_FAILURE;
+		}
+	}
+
+	close(fd);
+	exit(retval);
+}
--=20
1.7.0.4

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" =
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] xfstests: add new getdents test

[Christoph Hellwig]

On Mon, Sep 12, 2011 at 03:19:07AM +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@gmail.com>

Thanks a lot!  I've applied it locally and will push it out as soon
as kernel.org is back up.

Re: [PATCH] xfstests: add new getdents test

[Alex Elder]

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@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@sgi.com>

. . .

> +#include <sys/syscall.h>
> +
> +struct linux_dirent64 {
> +	uint64_t	d_ino;
> +	uint64_t	d_off;
> +	unsigned short	d_reclen;
> +	unsigned char	d_type;
> +	char		d_name[0];
> +};
> +
> +#define BUF_SIZE 4096
> +#define HISTORY_LEN 1024
> +
> +static uint64_t d_off_histoty[HISTORY_LEN];
> +static uint64_t d_ino_histoty[HISTORY_LEN];

Is "histoty" intentional or a typo?

> +int
> +main(int argc, char *argv[])
> +{
> +	int fd, nread;
> +	char buf[BUF_SIZE];

. . .

> +
> +	/* check if seek works correctly */
> +	d = (struct linux_dirent64 *)buf;
> +	for (i = total - 1; i >= 0; i--)
> +	{
> +		lret = lseek(fd, i > 0 ? d_off_histoty[i - 1] : 0, SEEK_SET);
> +		if (lret == -1) {
> +			perror("lseek");
> +			exit(EXIT_FAILURE);
> +		}
> +
> +		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.

> +		if (nread == -1) {
> +			perror("getdents");
> +			exit(EXIT_FAILURE);
> +		}
> +

. . .

Re: [PATCH] xfstests: add new getdents test

[Grazvydas Ignotas]

On Thu, Sep 22, 2011 at 11:18 PM, Alex Elder <aelder@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@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@sgi.com>
>
> . . .
>
>> +#include <sys/syscall.h>
>> +
>> +struct linux_dirent64 {
>> + =A0 =A0 uint64_t =A0 =A0 =A0 =A0d_ino;
>> + =A0 =A0 uint64_t =A0 =A0 =A0 =A0d_off;
>> + =A0 =A0 unsigned short =A0d_reclen;
>> + =A0 =A0 unsigned char =A0 d_type;
>> + =A0 =A0 char =A0 =A0 =A0 =A0 =A0 =A0d_name[0];
>> +};
>> +
>> +#define BUF_SIZE 4096
>> +#define HISTORY_LEN 1024
>> +
>> +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.

>
>> +int
>> +main(int argc, char *argv[])
>> +{
>> + =A0 =A0 int fd, nread;
>> + =A0 =A0 char buf[BUF_SIZE];
>
> . . .
>
>> +
>> + =A0 =A0 /* check if seek works correctly */
>> + =A0 =A0 d =3D (struct linux_dirent64 *)buf;
>> + =A0 =A0 for (i =3D total - 1; i >=3D 0; i--)
>> + =A0 =A0 {
>> + =A0 =A0 =A0 =A0 =A0 =A0 lret =3D lseek(fd, i > 0 ? d_off_histoty[i=
 - 1] : 0, SEEK_SET);
>> + =A0 =A0 =A0 =A0 =A0 =A0 if (lret =3D=3D -1) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 perror("lseek");
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 exit(EXIT_FAILURE);
>> + =A0 =A0 =A0 =A0 =A0 =A0 }
>> +
>> + =A0 =A0 =A0 =A0 =A0 =A0 nread =3D syscall(SYS_getdents64, fd, buf,=
 BUF_SIZE);
>
> You could just use sizeof (struct linux_dirent_64) rather than
> BUF_SIZE here. =A0I 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..


--=20
Gra=FEvydas
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" =
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] xfstests: add new getdents test

[Alex Elder]

On Fri, 2011-09-23 at 16:03 +0300, Grazvydas Ignotas wrote:
> On Thu, Sep 22, 2011 at 11:18 PM, Alex Elder <aelder@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@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@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