[Top] [All Lists]

Re: [PATCH] xfs_io: add the lseek() SEEK_DATA/SEEK_HOLE support

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH] xfs_io: add the lseek() SEEK_DATA/SEEK_HOLE support
From: Mark Tinguely <tinguely@xxxxxxx>
Date: Tue, 23 Oct 2012 09:08:32 -0500
Cc: xfs@xxxxxxxxxxx, Christoph Hellwig <hch@xxxxxxxxxxxxx>
In-reply-to: <20121022232931.GD4291@dastard>
References: <20121022213759.033667921@xxxxxxx> <20121022213804.616209844@xxxxxxx> <20121022232931.GD4291@dastard>
User-agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:9.0) Gecko/20120122 Thunderbird/9.0
On 10/22/12 18:29, Dave Chinner wrote:
On Mon, Oct 22, 2012 at 04:38:00PM -0500, Mark Tinguely wrote:

Type            Offset
HOLE            630784

xfs_io>  lseek -r -d 0
Type            Offset
DATA            0
DATA            65536
DATA            524288
xfs_io>  lseek -r -h 0
Type            Offset
HOLE            16384
HOLE            131072
HOLE            1049576

xfs_io>  lseek -r -a 0
DATA            0
HOLE            16384
DATA            65536
HOLE            131072
DATA            524288
HOLE            1049576

Good idea.

+#define _LARGEFILE64_SOURCE     /* See feature_test_macros(7) */

That's defined by _GNU_SOURCE, which is set in the makefiles, so not
necessary here.

Okay, I think a couple of the header files are redundantly redundant too.

+static void
+       printf(_(
+" returns the next hole or data offset at or after the specified offset\n"
+" Example:\n"
+" 'lseek -d 512'  - offset of data at or following offset 512\n"
+" Repositions and returns the offset of either the next data or hole.\n"
+" There is an implied hole at the end of file. If the specified offset is\n"
+" past end of file, or there is no data past the specied offset, the offset\n"
+" -1 is returned.\n"

I'd prefer that "EOF" rather than "-1" is printed in this case.

sounds good.

<deleted mess of things to clean up>

Given that we only support pread and pwrite operations, the
repositioning of the file pointer is irrelevant so probably should
not be mentioned. If it was relevant, then we'd also need to support
the other seek modes to reposition the file pointer. So jsut
mentioning that it returns the offset of the next ... is probably
sufficient here.

agreed. I did not the other lseek() whence options for that very reason.



Thanks for the feedback.

PS. To Christoph: Yes, a test will be added.

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