[PATCH 1/3] xfs_repair: fix some potential null pointer deferences

[PATCH 1/3] xfs_repair: fix some potential null pointer deferences

[Darrick J. Wong]

Fix some potential NULL pointer deferences that Coverity pointed out,
and remove a trivial dead integer check.

Coverity-id: 1375789, 1375790, 1375791, 1375792
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 repair/phase5.c |    2 +-
 repair/rmap.c   |    2 +-
 repair/slab.h   |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)


diff --git a/repair/phase5.c b/repair/phase5.c
index 3604d1d..cbda556 100644
--- a/repair/phase5.c
+++ b/repair/phase5.c
@@ -1925,7 +1925,7 @@ _("Insufficient memory to construct refcount cursor."));
 	refc_rec = pop_slab_cursor(refc_cur);
 	lptr = &btree_curs->level[0];
 
-	for (i = 0; i < lptr->num_blocks; i++)  {
+	for (i = 0; i < lptr->num_blocks && refc_rec != NULL; i++)  {
 		/*
 		 * block initialization, lay in block header
 		 */
diff --git a/repair/rmap.c b/repair/rmap.c
index 45e183a..7508973 100644
--- a/repair/rmap.c
+++ b/repair/rmap.c
@@ -790,7 +790,7 @@ compute_refcounts(
 		mark_inode_rl(mp, stack_top);
 
 		/* Set nbno to the bno of the next refcount change */
-		if (n < slab_count(rmaps))
+		if (n < slab_count(rmaps) && array_cur)
 			nbno = array_cur->rm_startblock;
 		else
 			nbno = NULLAGBLOCK;
diff --git a/repair/slab.h b/repair/slab.h
index 4aa5512..a2201f1 100644
--- a/repair/slab.h
+++ b/repair/slab.h
@@ -54,7 +54,7 @@ extern void *bag_item(struct xfs_bag *, size_t);
 
 #define foreach_bag_ptr_reverse(bag, idx, ptr) \
 	for ((idx) = bag_count(bag) - 1, (ptr) = bag_item((bag), (idx)); \
-	     (idx) >= 0 && (ptr) != NULL; \
+	     (ptr) != NULL; \
 	     (idx)--, (ptr) = bag_item((bag), (idx)))
 
 #endif /* SLAB_H_ */

[PATCH 2/3] xfs_repair: fix bogus rmapbt record owner check

[Darrick J. Wong]

Make the reverse mapping owner check actually validate inode numbers.

Coverity-id: 1371628
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 repair/scan.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)


diff --git a/repair/scan.c b/repair/scan.c
index 0e13581..b9ef4dc 100644
--- a/repair/scan.c
+++ b/repair/scan.c
@@ -1052,8 +1052,12 @@ _("%s rmap btree block claimed (state %d), agno %d, bno %d, suspect %d\n"),
 			}
 
 			/* Look for impossible owners. */
-			if (!(owner > 0 || (owner > XFS_RMAP_OWN_MIN &&
-					    owner <= XFS_RMAP_OWN_FS)))
+			if (!((owner > XFS_RMAP_OWN_MIN &&
+			       owner <= XFS_RMAP_OWN_FS) ||
+			      (XFS_INO_TO_AGNO(mp, owner) < mp->m_sb.sb_agcount &&
+			       XFS_AGINO_TO_AGBNO(mp,
+					XFS_INO_TO_AGINO(mp, owner)) <
+					mp->m_sb.sb_agblocks)))
 				do_warn(
 	_("invalid owner in rmap btree record %d (%"PRId64" %u) block %u/%u\n"),
 						i, owner, len, agno, bno);

[PATCH 3/3] xfs_io: fix the minimum arguments to the reflink command

[Darrick J. Wong]

The reflink command can reflink the entirety of two files if the
offsets and lengths are not specified... but we forgot to permit
that case.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 io/reflink.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/io/reflink.c b/io/reflink.c
index a09e82d..d26cbdd 100644
--- a/io/reflink.c
+++ b/io/reflink.c
@@ -302,7 +302,7 @@ reflink_init(void)
 	reflink_cmd.name = "reflink";
 	reflink_cmd.altname = "rl";
 	reflink_cmd.cfunc = reflink_f;
-	reflink_cmd.argmin = 4;
+	reflink_cmd.argmin = 1;
 	reflink_cmd.argmax = -1;
 	reflink_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
 	reflink_cmd.args =

Re: [PATCH 3/3] xfs_io: fix the minimum arguments to the reflink command

[Eric Sandeen]

On 12/15/16 12:11 PM, Darrick J. Wong wrote:
> The reflink command can reflink the entirety of two files if the
> offsets and lengths are not specified... but we forgot to permit
> that case.

Could you do me a favor and also make that clear in the help output?
The manpage says:

       reflink  [ -C ] [ -q ] src_file [src_offset dst_offset length]


but the short help doesn't indicate that it's optional:

reflink infile src_off dst_off len -- reflinks a number of bytes at a specified offset

and neither it nor the long help mentions the -C & -q switches:

xfs_io> help reflink
reflink infile src_off dst_off len -- reflinks a number of bytes at a specified offset

 Links a range of bytes (in block size increments) from a file into a range
 of bytes in the open file.  The two extent ranges need not contain identical
 data.

 Example:
 'reflink some_file 0 4096 32768' - links 32768 bytes from some_file at
                                    offset 0 to into the open file at
                                    position 4096
 'reflink some_file' - links all bytes from some_file into the open file
                       at position 0

 Reflink a range of blocks from a given input file to the open file.  Both
 files share the same range of physical disk blocks; a write to the shared
 range of either file should result in the write landing in a new block and
 that range of the file being remapped (i.e. copy-on-write).  Both files
 must reside on the same filesystem.

V2 or patch 4/3 is fine :)

Thanks,
-Eric

> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  io/reflink.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 
> diff --git a/io/reflink.c b/io/reflink.c
> index a09e82d..d26cbdd 100644
> --- a/io/reflink.c
> +++ b/io/reflink.c
> @@ -302,7 +302,7 @@ reflink_init(void)
>  	reflink_cmd.name = "reflink";
>  	reflink_cmd.altname = "rl";
>  	reflink_cmd.cfunc = reflink_f;
> -	reflink_cmd.argmin = 4;
> +	reflink_cmd.argmin = 1;
>  	reflink_cmd.argmax = -1;
>  	reflink_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
>  	reflink_cmd.args =
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 2/3] xfs_repair: fix bogus rmapbt record owner check

[Eric Sandeen]

On 12/15/16 12:11 PM, Darrick J. Wong wrote:
> Make the reverse mapping owner check actually validate inode numbers.

Thanks, you also sent this on 11/4 so it's in my stack already.

-Eric
 
> Coverity-id: 1371628
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  repair/scan.c |    8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/repair/scan.c b/repair/scan.c
> index 0e13581..b9ef4dc 100644
> --- a/repair/scan.c
> +++ b/repair/scan.c
> @@ -1052,8 +1052,12 @@ _("%s rmap btree block claimed (state %d), agno %d, bno %d, suspect %d\n"),
>  			}
>  
>  			/* Look for impossible owners. */
> -			if (!(owner > 0 || (owner > XFS_RMAP_OWN_MIN &&
> -					    owner <= XFS_RMAP_OWN_FS)))
> +			if (!((owner > XFS_RMAP_OWN_MIN &&
> +			       owner <= XFS_RMAP_OWN_FS) ||
> +			      (XFS_INO_TO_AGNO(mp, owner) < mp->m_sb.sb_agcount &&
> +			       XFS_AGINO_TO_AGBNO(mp,
> +					XFS_INO_TO_AGINO(mp, owner)) <
> +					mp->m_sb.sb_agblocks)))
>  				do_warn(
>  	_("invalid owner in rmap btree record %d (%"PRId64" %u) block %u/%u\n"),
>  						i, owner, len, agno, bno);
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 1/3] xfs_repair: fix some potential null pointer deferences

[Eric Sandeen]

On 12/15/16 12:11 PM, Darrick J. Wong wrote:
> Fix some potential NULL pointer deferences that Coverity pointed out,
> and remove a trivial dead integer check.
> 
> Coverity-id: 1375789, 1375790, 1375791, 1375792
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  repair/phase5.c |    2 +-
>  repair/rmap.c   |    2 +-
>  repair/slab.h   |    2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> 
> diff --git a/repair/phase5.c b/repair/phase5.c
> index 3604d1d..cbda556 100644
> --- a/repair/phase5.c
> +++ b/repair/phase5.c
> @@ -1925,7 +1925,7 @@ _("Insufficient memory to construct refcount cursor."));
>  	refc_rec = pop_slab_cursor(refc_cur);
>  	lptr = &btree_curs->level[0];
>  
> -	for (i = 0; i < lptr->num_blocks; i++)  {
> +	for (i = 0; i < lptr->num_blocks && refc_rec != NULL; i++)  {

Ok, you sent this patch already as well :)

And it matches the exact same thing that exists for a while in
the rmap_tree builder, so yay for that.  :)

but is this just shutting up coverity, or is this something that could
happen?  If it /does/ happen, is skipping the loop all we really need
to do?  Sorry - I have to think harder about what's going on in here
as we build the tree, but maybe you already know. 

My sense of tidiness and consistency is bothered by the fact that
we don't have similar null-check-skip-loop for, say, ino_rec
in build_ino_tree, or ext_ptr in build_freespace_tree.


>  		/*
>  		 * block initialization, lay in block header
>  		 */
> diff --git a/repair/rmap.c b/repair/rmap.c
> index 45e183a..7508973 100644
> --- a/repair/rmap.c
> +++ b/repair/rmap.c
> @@ -790,7 +790,7 @@ compute_refcounts(
>  		mark_inode_rl(mp, stack_top);
>  
>  		/* Set nbno to the bno of the next refcount change */
> -		if (n < slab_count(rmaps))
> +		if (n < slab_count(rmaps) && array_cur)
>  			nbno = array_cur->rm_startblock;
>  		else
>  			nbno = NULLAGBLOCK;
> diff --git a/repair/slab.h b/repair/slab.h
> index 4aa5512..a2201f1 100644
> --- a/repair/slab.h
> +++ b/repair/slab.h
> @@ -54,7 +54,7 @@ extern void *bag_item(struct xfs_bag *, size_t);
>  
>  #define foreach_bag_ptr_reverse(bag, idx, ptr) \
>  	for ((idx) = bag_count(bag) - 1, (ptr) = bag_item((bag), (idx)); \
> -	     (idx) >= 0 && (ptr) != NULL; \
> +	     (ptr) != NULL; \
>  	     (idx)--, (ptr) = bag_item((bag), (idx)))
>  
>  #endif /* SLAB_H_ */
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 3/3] xfs_io: fix the minimum arguments to the reflink command

[Darrick J. Wong]

On Thu, Dec 15, 2016 at 12:38:34PM -0600, Eric Sandeen wrote:
> On 12/15/16 12:11 PM, Darrick J. Wong wrote:
> > The reflink command can reflink the entirety of two files if the
> > offsets and lengths are not specified... but we forgot to permit
> > that case.
> 
> Could you do me a favor and also make that clear in the help output?
> The manpage says:
> 
>        reflink  [ -C ] [ -q ] src_file [src_offset dst_offset length]
> 
> 
> but the short help doesn't indicate that it's optional:
> 
> reflink infile src_off dst_off len -- reflinks a number of bytes at a specified offset

Hah, ok, yes, that's unclear and I'll fix it in a 4/3 patch.

--D

> 
> and neither it nor the long help mentions the -C & -q switches:
> 
> xfs_io> help reflink
> reflink infile src_off dst_off len -- reflinks a number of bytes at a specified offset
> 
>  Links a range of bytes (in block size increments) from a file into a range
>  of bytes in the open file.  The two extent ranges need not contain identical
>  data.
> 
>  Example:
>  'reflink some_file 0 4096 32768' - links 32768 bytes from some_file at
>                                     offset 0 to into the open file at
>                                     position 4096
>  'reflink some_file' - links all bytes from some_file into the open file
>                        at position 0
> 
>  Reflink a range of blocks from a given input file to the open file.  Both
>  files share the same range of physical disk blocks; a write to the shared
>  range of either file should result in the write landing in a new block and
>  that range of the file being remapped (i.e. copy-on-write).  Both files
>  must reside on the same filesystem.
> 
> V2 or patch 4/3 is fine :)
> 
> Thanks,
> -Eric
> 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  io/reflink.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > 
> > diff --git a/io/reflink.c b/io/reflink.c
> > index a09e82d..d26cbdd 100644
> > --- a/io/reflink.c
> > +++ b/io/reflink.c
> > @@ -302,7 +302,7 @@ reflink_init(void)
> >  	reflink_cmd.name = "reflink";
> >  	reflink_cmd.altname = "rl";
> >  	reflink_cmd.cfunc = reflink_f;
> > -	reflink_cmd.argmin = 4;
> > +	reflink_cmd.argmin = 1;
> >  	reflink_cmd.argmax = -1;
> >  	reflink_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
> >  	reflink_cmd.args =
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/3] xfs_io: fix some documentation problems

[Darrick J. Wong]

Describe the numberless (i.e. "reflink the whole file") behavior
in the xfs_io help system and since the clone/dedupe ioctls were
promoted to the VFS before XFS reflink landed, mention those in
the manpage.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 io/reflink.c      |    4 ++--
 man/man8/xfs_io.8 |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/io/reflink.c b/io/reflink.c
index 7b15e89..d4c745c 100644
--- a/io/reflink.c
+++ b/io/reflink.c
@@ -307,9 +307,9 @@ reflink_init(void)
 	reflink_cmd.argmax = -1;
 	reflink_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
 	reflink_cmd.args =
-_("infile src_off dst_off len");
+_("infile [src_off dst_off len]");
 	reflink_cmd.oneline =
-		_("reflinks a number of bytes at a specified offset");
+		_("reflinks an entire file, or a number of bytes at a specified offset");
 	reflink_cmd.help = reflink_help;
 
 	add_command(&reflink_cmd);
diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
index f5e89ab..5cea992 100644
--- a/man/man8/xfs_io.8
+++ b/man/man8/xfs_io.8
@@ -577,7 +577,7 @@ both data and holes are displayed together or performing a recusively display.
 .TP
 .BI "reflink  [ \-C ] [ \-q ] src_file [src_offset dst_offset length]"
 On filesystems that support the
-.B XFS_IOC_CLONE_RANGE
+.B FICLONERANGE
 or
 .B BTRFS_IOC_CLONE_RANGE
 ioctls, map
@@ -607,7 +607,7 @@ Do not print timing statistics at all.
 .TP
 .BI "dedupe  [ \-C ] [ \-q ] src_file src_offset dst_offset length"
 On filesystems that support the
-.B XFS_IOC_FILE_EXTENT_SAME
+.B FIDEDUPERANGE
 or
 .B BTRFS_IOC_FILE_EXTENT_SAME
 ioctls, map

Re: [PATCH 1/3] xfs_repair: fix some potential null pointer deferences

[Darrick J. Wong]

On Thu, Dec 15, 2016 at 05:17:14PM -0600, Eric Sandeen wrote:
> On 12/15/16 12:11 PM, Darrick J. Wong wrote:
> > Fix some potential NULL pointer deferences that Coverity pointed out,
> > and remove a trivial dead integer check.
> > 
> > Coverity-id: 1375789, 1375790, 1375791, 1375792
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  repair/phase5.c |    2 +-
> >  repair/rmap.c   |    2 +-
> >  repair/slab.h   |    2 +-
> >  3 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > 
> > diff --git a/repair/phase5.c b/repair/phase5.c
> > index 3604d1d..cbda556 100644
> > --- a/repair/phase5.c
> > +++ b/repair/phase5.c
> > @@ -1925,7 +1925,7 @@ _("Insufficient memory to construct refcount cursor."));
> >  	refc_rec = pop_slab_cursor(refc_cur);
> >  	lptr = &btree_curs->level[0];
> >  
> > -	for (i = 0; i < lptr->num_blocks; i++)  {
> > +	for (i = 0; i < lptr->num_blocks && refc_rec != NULL; i++)  {
> 
> Ok, you sent this patch already as well :)
> 
> And it matches the exact same thing that exists for a while in
> the rmap_tree builder, so yay for that.  :)
> 
> but is this just shutting up coverity, or is this something that could
> happen?  If it /does/ happen, is skipping the loop all we really need
> to do?  Sorry - I have to think harder about what's going on in here
> as we build the tree, but maybe you already know. 

It's just shutting Coverity up -- we should always run out of num_blocks
before pop_slab_cursor runs out of records to put in the leaf blocks and
returns NULL.  init_{rmap,refcount}bt_cursor does the following:

num_recs = {rmap,refcount}_record_count(...);
lptr->num_blocks = (num_recs + maxrecs - 1) / maxrecs;

But there's no harm in sanity checking just in case this ever becomes
untrue.

> My sense of tidiness and consistency is bothered by the fact that
> we don't have similar null-check-skip-loop for, say, ino_rec
> in build_ino_tree, or ext_ptr in build_freespace_tree.

The other tree builders don't use slabs, so the loops are different.

--D

> 
> 
> >  		/*
> >  		 * block initialization, lay in block header
> >  		 */
> > diff --git a/repair/rmap.c b/repair/rmap.c
> > index 45e183a..7508973 100644
> > --- a/repair/rmap.c
> > +++ b/repair/rmap.c
> > @@ -790,7 +790,7 @@ compute_refcounts(
> >  		mark_inode_rl(mp, stack_top);
> >  
> >  		/* Set nbno to the bno of the next refcount change */
> > -		if (n < slab_count(rmaps))
> > +		if (n < slab_count(rmaps) && array_cur)
> >  			nbno = array_cur->rm_startblock;
> >  		else
> >  			nbno = NULLAGBLOCK;
> > diff --git a/repair/slab.h b/repair/slab.h
> > index 4aa5512..a2201f1 100644
> > --- a/repair/slab.h
> > +++ b/repair/slab.h
> > @@ -54,7 +54,7 @@ extern void *bag_item(struct xfs_bag *, size_t);
> >  
> >  #define foreach_bag_ptr_reverse(bag, idx, ptr) \
> >  	for ((idx) = bag_count(bag) - 1, (ptr) = bag_item((bag), (idx)); \
> > -	     (idx) >= 0 && (ptr) != NULL; \
> > +	     (ptr) != NULL; \
> >  	     (idx)--, (ptr) = bag_item((bag), (idx)))
> >  
> >  #endif /* SLAB_H_ */
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] xfs_repair: fix some potential null pointer deferences

[Eric Sandeen]

On 12/15/16 6:58 PM, Darrick J. Wong wrote:
> On Thu, Dec 15, 2016 at 05:17:14PM -0600, Eric Sandeen wrote:
>> On 12/15/16 12:11 PM, Darrick J. Wong wrote:
>>> Fix some potential NULL pointer deferences that Coverity pointed out,
>>> and remove a trivial dead integer check.
>>>
>>> Coverity-id: 1375789, 1375790, 1375791, 1375792
>>> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
>>> ---
>>>  repair/phase5.c |    2 +-
>>>  repair/rmap.c   |    2 +-
>>>  repair/slab.h   |    2 +-
>>>  3 files changed, 3 insertions(+), 3 deletions(-)
>>>
>>>
>>> diff --git a/repair/phase5.c b/repair/phase5.c
>>> index 3604d1d..cbda556 100644
>>> --- a/repair/phase5.c
>>> +++ b/repair/phase5.c
>>> @@ -1925,7 +1925,7 @@ _("Insufficient memory to construct refcount cursor."));
>>>  	refc_rec = pop_slab_cursor(refc_cur);
>>>  	lptr = &btree_curs->level[0];
>>>  
>>> -	for (i = 0; i < lptr->num_blocks; i++)  {
>>> +	for (i = 0; i < lptr->num_blocks && refc_rec != NULL; i++)  {
>>
>> Ok, you sent this patch already as well :)
>>
>> And it matches the exact same thing that exists for a while in
>> the rmap_tree builder, so yay for that.  :)
>>
>> but is this just shutting up coverity, or is this something that could
>> happen?  If it /does/ happen, is skipping the loop all we really need
>> to do?  Sorry - I have to think harder about what's going on in here
>> as we build the tree, but maybe you already know. 
> 
> It's just shutting Coverity up -- we should always run out of num_blocks
> before pop_slab_cursor runs out of records to put in the leaf blocks and
> returns NULL.  init_{rmap,refcount}bt_cursor does the following:
> 
> num_recs = {rmap,refcount}_record_count(...);
> lptr->num_blocks = (num_recs + maxrecs - 1) / maxrecs;
> 
> But there's no harm in sanity checking just in case this ever becomes
> untrue.

I wonder if an ASSERT would also shut Coverity up - that'd communicate
to coverity that we can't ever deref a null ptr, but also communicates
to the reader that a null ptr is not expected (better than a test
in a loop, which looks like maybe it is expected...)

/me wishes covscan allowed scratch builds to test

-Eric

>> My sense of tidiness and consistency is bothered by the fact that
>> we don't have similar null-check-skip-loop for, say, ino_rec
>> in build_ino_tree, or ext_ptr in build_freespace_tree.
> 
> The other tree builders don't use slabs, so the loops are different.
> 
> --D
> 
>>
>>
>>>  		/*
>>>  		 * block initialization, lay in block header
>>>  		 */
>>> diff --git a/repair/rmap.c b/repair/rmap.c
>>> index 45e183a..7508973 100644
>>> --- a/repair/rmap.c
>>> +++ b/repair/rmap.c
>>> @@ -790,7 +790,7 @@ compute_refcounts(
>>>  		mark_inode_rl(mp, stack_top);
>>>  
>>>  		/* Set nbno to the bno of the next refcount change */
>>> -		if (n < slab_count(rmaps))
>>> +		if (n < slab_count(rmaps) && array_cur)
>>>  			nbno = array_cur->rm_startblock;
>>>  		else
>>>  			nbno = NULLAGBLOCK;
>>> diff --git a/repair/slab.h b/repair/slab.h
>>> index 4aa5512..a2201f1 100644
>>> --- a/repair/slab.h
>>> +++ b/repair/slab.h
>>> @@ -54,7 +54,7 @@ extern void *bag_item(struct xfs_bag *, size_t);
>>>  
>>>  #define foreach_bag_ptr_reverse(bag, idx, ptr) \
>>>  	for ((idx) = bag_count(bag) - 1, (ptr) = bag_item((bag), (idx)); \
>>> -	     (idx) >= 0 && (ptr) != NULL; \
>>> +	     (ptr) != NULL; \
>>>  	     (idx)--, (ptr) = bag_item((bag), (idx)))
>>>  
>>>  #endif /* SLAB_H_ */
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 3/3] xfs_io: fix the minimum arguments to the reflink command

[Eric Sandeen]

On 12/15/16 12:11 PM, Darrick J. Wong wrote:
> The reflink command can reflink the entirety of two files if the
> offsets and lengths are not specified... but we forgot to permit
> that case.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

and thanks for the 4/3 patch.

> ---
>  io/reflink.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 
> diff --git a/io/reflink.c b/io/reflink.c
> index a09e82d..d26cbdd 100644
> --- a/io/reflink.c
> +++ b/io/reflink.c
> @@ -302,7 +302,7 @@ reflink_init(void)
>  	reflink_cmd.name = "reflink";
>  	reflink_cmd.altname = "rl";
>  	reflink_cmd.cfunc = reflink_f;
> -	reflink_cmd.argmin = 4;
> +	reflink_cmd.argmin = 1;
>  	reflink_cmd.argmax = -1;
>  	reflink_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
>  	reflink_cmd.args =
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 4/3] xfs_io: fix some documentation problems

[Eric Sandeen]

On 12/15/16 6:52 PM, Darrick J. Wong wrote:
> Describe the numberless (i.e. "reflink the whole file") behavior
> in the xfs_io help system and since the clone/dedupe ioctls were
> promoted to the VFS before XFS reflink landed, mention those in
> the manpage.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
>  io/reflink.c      |    4 ++--
>  man/man8/xfs_io.8 |    4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/io/reflink.c b/io/reflink.c
> index 7b15e89..d4c745c 100644
> --- a/io/reflink.c
> +++ b/io/reflink.c
> @@ -307,9 +307,9 @@ reflink_init(void)
>  	reflink_cmd.argmax = -1;
>  	reflink_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
>  	reflink_cmd.args =
> -_("infile src_off dst_off len");
> +_("infile [src_off dst_off len]");
>  	reflink_cmd.oneline =
> -		_("reflinks a number of bytes at a specified offset");
> +		_("reflinks an entire file, or a number of bytes at a specified offset");
>  	reflink_cmd.help = reflink_help;
>  
>  	add_command(&reflink_cmd);
> diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
> index f5e89ab..5cea992 100644
> --- a/man/man8/xfs_io.8
> +++ b/man/man8/xfs_io.8
> @@ -577,7 +577,7 @@ both data and holes are displayed together or performing a recusively display.
>  .TP
>  .BI "reflink  [ \-C ] [ \-q ] src_file [src_offset dst_offset length]"
>  On filesystems that support the
> -.B XFS_IOC_CLONE_RANGE
> +.B FICLONERANGE
>  or
>  .B BTRFS_IOC_CLONE_RANGE
>  ioctls, map
> @@ -607,7 +607,7 @@ Do not print timing statistics at all.
>  .TP
>  .BI "dedupe  [ \-C ] [ \-q ] src_file src_offset dst_offset length"
>  On filesystems that support the
> -.B XFS_IOC_FILE_EXTENT_SAME
> +.B FIDEDUPERANGE
>  or
>  .B BTRFS_IOC_FILE_EXTENT_SAME
>  ioctls, map
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>