[01/10] xfs: change xfs_attr3_rmt_hdr_ok to return bool
diff mbox series

Message ID 1b1b2992-5304-e9e2-b20e-ac41235744f7@redhat.com
State New
Headers show
Series
  • xfs: add verifier context structure
Related show

Commit Message

Eric Sandeen Dec. 5, 2018, 9:02 p.m. UTC
xfs_attr3_rmt_hdr_ok is only checked for true/false and the caller
does nothing with the returned failaddr, so make it a bool.

(its usage and naming lend itself to a bool too, i.e. test (!ok))

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 fs/xfs/libxfs/xfs_attr_remote.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Comments

Brian Foster Dec. 7, 2018, 1:36 p.m. UTC | #1
On Wed, Dec 05, 2018 at 03:02:15PM -0600, Eric Sandeen wrote:
> xfs_attr3_rmt_hdr_ok is only checked for true/false and the caller
> does nothing with the returned failaddr, so make it a bool.
> 
> (its usage and naming lend itself to a bool too, i.e. test (!ok))
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/libxfs/xfs_attr_remote.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
> index d89363c6b523..f86680e08613 100644
> --- a/fs/xfs/libxfs/xfs_attr_remote.c
> +++ b/fs/xfs/libxfs/xfs_attr_remote.c
> @@ -53,7 +53,7 @@ xfs_attr3_rmt_blocks(
>   * does CRC, location and bounds checking, the unpacking function checks the
>   * attribute parameters and owner.
>   */
> -static xfs_failaddr_t
> +static bool
>  xfs_attr3_rmt_hdr_ok(
>  	void			*ptr,
>  	xfs_ino_t		ino,
> @@ -64,16 +64,16 @@ xfs_attr3_rmt_hdr_ok(
>  	struct xfs_attr3_rmt_hdr *rmt = ptr;
>  
>  	if (bno != be64_to_cpu(rmt->rm_blkno))
> -		return __this_address;
> +		return false;
>  	if (offset != be32_to_cpu(rmt->rm_offset))
> -		return __this_address;
> +		return false;
>  	if (size != be32_to_cpu(rmt->rm_bytes))
> -		return __this_address;
> +		return false;
>  	if (ino != be64_to_cpu(rmt->rm_owner))
> -		return __this_address;
> +		return false;
>  
>  	/* ok */
> -	return NULL;
> +	return true;
>  }
>  
>  static xfs_failaddr_t
> @@ -287,7 +287,7 @@ xfs_attr_rmtval_copyout(
>  		byte_cnt = min(*valuelen, byte_cnt);
>  
>  		if (xfs_sb_version_hascrc(&mp->m_sb)) {
> -			if (xfs_attr3_rmt_hdr_ok(src, ino, *offset,
> +			if (!xfs_attr3_rmt_hdr_ok(src, ino, *offset,
>  						  byte_cnt, bno)) {
>  				xfs_alert(mp,
>  "remote attribute header mismatch bno/off/len/owner (0x%llx/0x%x/Ox%x/0x%llx)",
> -- 
> 2.17.0
> 
>
Darrick J. Wong Dec. 17, 2018, 6:23 p.m. UTC | #2
On Wed, Dec 05, 2018 at 03:02:15PM -0600, Eric Sandeen wrote:
> xfs_attr3_rmt_hdr_ok is only checked for true/false and the caller
> does nothing with the returned failaddr, so make it a bool.
> (its usage and naming lend itself to a bool too, i.e. test (!ok))
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>  fs/xfs/libxfs/xfs_attr_remote.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
> index d89363c6b523..f86680e08613 100644
> --- a/fs/xfs/libxfs/xfs_attr_remote.c
> +++ b/fs/xfs/libxfs/xfs_attr_remote.c
> @@ -53,7 +53,7 @@ xfs_attr3_rmt_blocks(
>   * does CRC, location and bounds checking, the unpacking function checks the
>   * attribute parameters and owner.
>   */
> -static xfs_failaddr_t
> +static bool
>  xfs_attr3_rmt_hdr_ok(
>  	void			*ptr,
>  	xfs_ino_t		ino,
> @@ -64,16 +64,16 @@ xfs_attr3_rmt_hdr_ok(
>  	struct xfs_attr3_rmt_hdr *rmt = ptr;
>  
>  	if (bno != be64_to_cpu(rmt->rm_blkno))
> -		return __this_address;
> +		return false;
>  	if (offset != be32_to_cpu(rmt->rm_offset))
> -		return __this_address;
> +		return false;
>  	if (size != be32_to_cpu(rmt->rm_bytes))
> -		return __this_address;
> +		return false;
>  	if (ino != be64_to_cpu(rmt->rm_owner))
> -		return __this_address;
> +		return false;
>  
>  	/* ok */
> -	return NULL;
> +	return true;
>  }
>  
>  static xfs_failaddr_t
> @@ -287,7 +287,7 @@ xfs_attr_rmtval_copyout(
>  		byte_cnt = min(*valuelen, byte_cnt);
>  
>  		if (xfs_sb_version_hascrc(&mp->m_sb)) {
> -			if (xfs_attr3_rmt_hdr_ok(src, ino, *offset,
> +			if (!xfs_attr3_rmt_hdr_ok(src, ino, *offset,

Why not pass a verifier context to attr3_rmt_hdr_ok and enable
rmtval_copyout to report the approximate instruction address of the
failed check?

(Or maybe I missed you doing this later...?)

--D

>  						  byte_cnt, bno)) {
>  				xfs_alert(mp,
>  "remote attribute header mismatch bno/off/len/owner (0x%llx/0x%x/Ox%x/0x%llx)",
> -- 
> 2.17.0
> 
>

Patch
diff mbox series

diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
index d89363c6b523..f86680e08613 100644
--- a/fs/xfs/libxfs/xfs_attr_remote.c
+++ b/fs/xfs/libxfs/xfs_attr_remote.c
@@ -53,7 +53,7 @@  xfs_attr3_rmt_blocks(
  * does CRC, location and bounds checking, the unpacking function checks the
  * attribute parameters and owner.
  */
-static xfs_failaddr_t
+static bool
 xfs_attr3_rmt_hdr_ok(
 	void			*ptr,
 	xfs_ino_t		ino,
@@ -64,16 +64,16 @@  xfs_attr3_rmt_hdr_ok(
 	struct xfs_attr3_rmt_hdr *rmt = ptr;
 
 	if (bno != be64_to_cpu(rmt->rm_blkno))
-		return __this_address;
+		return false;
 	if (offset != be32_to_cpu(rmt->rm_offset))
-		return __this_address;
+		return false;
 	if (size != be32_to_cpu(rmt->rm_bytes))
-		return __this_address;
+		return false;
 	if (ino != be64_to_cpu(rmt->rm_owner))
-		return __this_address;
+		return false;
 
 	/* ok */
-	return NULL;
+	return true;
 }
 
 static xfs_failaddr_t
@@ -287,7 +287,7 @@  xfs_attr_rmtval_copyout(
 		byte_cnt = min(*valuelen, byte_cnt);
 
 		if (xfs_sb_version_hascrc(&mp->m_sb)) {
-			if (xfs_attr3_rmt_hdr_ok(src, ino, *offset,
+			if (!xfs_attr3_rmt_hdr_ok(src, ino, *offset,
 						  byte_cnt, bno)) {
 				xfs_alert(mp,
 "remote attribute header mismatch bno/off/len/owner (0x%llx/0x%x/Ox%x/0x%llx)",