On Thu, Nov 07, 2019 at 02:41:57PM -0800, Darrick J. Wong wrote:
> On Thu, Nov 07, 2019 at 07:23:54PM +0100, Christoph Hellwig wrote:
> > Use an offset as the main means for iteration, and only do pointer
> > arithmetics to find the data/unused entries.
> >
> > Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> > ---
> > fs/xfs/xfs_dir2_readdir.c | 34 +++++++++++++++++-----------------
> > 1 file changed, 17 insertions(+), 17 deletions(-)
> >
> > diff --git a/fs/xfs/xfs_dir2_readdir.c b/fs/xfs/xfs_dir2_readdir.c
> > index 0d234b649d65..c4314e9e3dd8 100644
> > --- a/fs/xfs/xfs_dir2_readdir.c
> > +++ b/fs/xfs/xfs_dir2_readdir.c
> > @@ -351,13 +351,13 @@ xfs_dir2_leaf_getdents(
> > xfs_dir2_data_hdr_t *hdr; /* data block header */
>
> gcc complained about this variable being set but not used.
Your gcc is obviously smarted than mine, because it is unused.
>
> > xfs_dir2_data_entry_t *dep; /* data entry */
> > xfs_dir2_data_unused_t *dup; /* unused entry */
> > - char *ptr = NULL; /* pointer to current data */
> > struct xfs_da_geometry *geo = args->geo;
> > xfs_dablk_t rablk = 0; /* current readahead block */
> > xfs_dir2_off_t curoff; /* current overall offset */
> > int length; /* temporary length value */
> > int byteoff; /* offset in current block */
> > int lock_mode;
> > + unsigned int offset = 0;
>
> This is the offset within the block, right?
Yes.
>
> > int error = 0; /* error return value */
> >
> > /*
> > @@ -384,7 +384,7 @@ xfs_dir2_leaf_getdents(
> > * If we have no buffer, or we're off the end of the
> > * current buffer, need to get another one.
> > */
> > - if (!bp || ptr >= (char *)bp->b_addr + geo->blksize) {
> > + if (!bp || offset + geo->blksize) {
>
> ^^^^^^^^^^^^^^^^^^^^^
>
> In which case, isn't this always true? Was this supposed to be
> offset >= geo->blksize?
Yes, fixed up now.
