xfs
[Top] [All Lists]

Re: [PATCH] xfstests: add new getdents test

To: aelder@xxxxxxx
Subject: Re: [PATCH] xfstests: add new getdents test
From: Grazvydas Ignotas <notasas@xxxxxxxxx>
Date: Fri, 23 Sep 2011 16:03:41 +0300
Cc: xfs@xxxxxxxxxxx, linux-btrfs@xxxxxxxxxxxxxxx
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; bh=2Ec3lTJCazrFluYJKK74PegePrumnafWJpIOVhg7aHQ=; b=DqF77ZTeGYXfkuWYpXEzWK3R1/JL26CK9dgOhWx1NDGBxBkBJ89zg50Fv1R3R5vb16 3G9+5w1gY08Y3zqckP+lkxZX9Y+1hEbwTDUpuG4gLXyv8bQuguALpCfzvdMxA1p2j+BW 2Aw6Z2o+iM/BDpVgDwuLBggKEXn8RzEUr74RM=
In-reply-to: <1316722702.2009.59.camel@doink>
References: <1315786747-12109-1-git-send-email-notasas@xxxxxxxxx> <1316722702.2009.59.camel@doink>
On Thu, Sep 22, 2011 at 11:18 PM, Alex Elder <aelder@xxxxxxx> 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@xxxxxxxxx>
>
> 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@xxxxxxx>
>
> . . .
>
>> +#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?

whoops, it's a typo. I might send a patch for this later.

>
>> +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.

I'm not sure if reading partial entry is allowed, manpage says it may
fail with EINVAL if buffer size is too small..


-- 
Gražvydas

<Prev in Thread] Current Thread [Next in Thread>