xfs
[Top] [All Lists]

Re: [PATCH] xfsprogs/io: add getdents command

To: Zach Brown <zab@xxxxxxxxxx>
Subject: Re: [PATCH] xfsprogs/io: add getdents command
From: Brian Foster <bfoster@xxxxxxxxxx>
Date: Tue, 23 Jul 2013 11:01:29 -0400
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20130715183845.GS25414@xxxxxxxxxxxxxxxxxxxx>
References: <1373567034-7534-1-git-send-email-bfoster@xxxxxxxxxx> <20130715183845.GS25414@xxxxxxxxxxxxxxxxxxxx>
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7
On 07/15/2013 02:38 PM, Zach Brown wrote:
>> +ifeq ($(PKG_PLATFORM),linux)
>> +CFILES += getdents.c
>> +endif
> 
> I'd make a real test for getdents() rather than tying it to Linux.  Just
> copy what's done for sync_file_range :). 
> 

Having a look back, I think a reason why I avoided this was I didn't
have a libc interface for getdents. Perhaps there's still a way to test
that, but I'm also wondering if this would be more broadly useful if I
just converted it over to generic readdir(). Thoughts?

Brian

>> +struct linux_dirent {
>> +    unsigned long   d_ino;     /* Inode number */
>> +    unsigned long   d_off;     /* Offset to next linux_dirent */
>> +    unsigned short  d_reclen;  /* Length of this linux_dirent */
>> +    char            d_name[];  /* Filename (null-terminated) */
>> +};
> 
> If we're only going to use one syscall interface, I'd use getdents64().
> 
> struct linux_dirent64 {
>         u64             d_ino;
>         s64             d_off;
>         unsigned short  d_reclen;
>         unsigned char   d_type;
>         char            d_name[0];
> };      
> 
> But it could also be useful to see the native 'long' interface for 32bit
> apps, though, say if you're trying to debug ext4 htree d_off values
> which are magically truncated for 32bit compat tasks.
> 
>> +static void
>> +dump_dir_buffer(
>> +    struct linux_dirent *dirp,
>> +    long long offset,
>> +    long long length)
>> +{
>> +    while (length > 0) {
>> +            printf("%08llx: 0x%lx (%s)\n",
>> +                    offset, dirp->d_ino, dirp->d_name);
>> +
>> +            offset = dirp->d_off;
>> +            length -= dirp->d_reclen;
>> +
>> +            dirp = (struct linux_dirent *) ((char *) dirp + dirp->d_reclen);
>> +    }
>> +}
> 
> You could also print out d_type.  In getdents64() it's a proper struct
> member, in the bad old 'long' interface it's hidden in padding after the
> null in d_name.
> 
> It'd be nice to see d_off for the last entry.  btrfs, for example, sets
> it to CRAZY and that can be good to know.  In the past it has caused
> 32bit glibc to return -EOVERFLOW when reading entries from getdents64()
> whose d_off overflowed 32bit off_t.
> 
> Interestingly, notice that with getdents() you don't know what the
> offset of the first entry is.  You just know the offset that you started
> reading from and the offset of the next entry.
> 
> - z
> (*throws another bag of nickels in the readdir-design-disaster jar*)
> 

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