xfs: fix compiler warnings

PATCH Superseded Darrick J. Wong 4 participants 8 messages Aug 31, 2017 – Sep 1, 2017 XFS filesystem

lore

Share & tools

lore

Darrick J. Wong

author

2017-08-31 23:39 UTC #0 lore raw

PATCH v2 1/1 xfs: fix compiler warnings

Fix up all the compiler warnings that have crept in.

Signed-off-by Darrick J. Wong <[email protected]>

---
 fs/xfs/libxfs/xfs_inode_fork.c |    9 +++------
 fs/xfs/xfs_buf_item.c          |    2 ++
 fs/xfs/xfs_log_recover.c       |    4 ++++
 3 files changed, 9 insertions(+), 6 deletions(-)

fs/xfs/libxfs/xfs_inode_fork.c +3 -6

index fb310d0..31840ca 100644
@@ -1499,14 +1499,11 @@ xfs_iext_realloc_indirect(
 	xfs_ifork_t	*ifp,		/* inode fork pointer */
 	int		new_size)	/* new indirection array size */
 {
-	int		nlists;		/* number of irec's (ex lists) */
-	int		size;		/* current indirection array size */
-
 	ASSERT(ifp->if_flags & XFS_IFEXTIREC);
-	nlists = ifp->if_real_bytes / XFS_IEXT_BUFSZ;
-	size = nlists * sizeof(xfs_ext_irec_t);
 	ASSERT(ifp->if_real_bytes);
-	ASSERT((new_size >= 0) && (new_size != size));
+	ASSERT((new_size >= 0) &&
+	       (new_size != ((ifp->if_real_bytes / XFS_IEXT_BUFSZ) *
+			     sizeof(xfs_ext_irec_t))));
 	if (new_size == 0) {
 		xfs_iext_destroy(ifp);
 	} else {

fs/xfs/xfs_buf_item.c +2 -0

index ef2c137..1a7814a 100644
@@ -570,7 +570,9 @@ xfs_buf_item_unlock(
 	bool			aborted = !!(lip->li_flags & XFS_LI_ABORTED);
 	bool			hold = !!(bip->bli_flags & XFS_BLI_HOLD);
 	bool			dirty = !!(bip->bli_flags & XFS_BLI_DIRTY);
+#ifdef DEBUG
 	bool			ordered = !!(bip->bli_flags & XFS_BLI_ORDERED);
+#endif
 
 	/* Clear the buffer's association with this transaction. */
 	bp->b_transp = NULL;

fs/xfs/xfs_log_recover.c +4 -0

index a362399..04a131c 100644
@@ -4827,12 +4827,16 @@ xlog_recover_process_intents(
 	int			error = 0;
 	struct xfs_ail_cursor	cur;
 	struct xfs_ail		*ailp;
+#ifdef DEBUG
 	xfs_lsn_t		last_lsn;
+#endif
 
 	ailp = log->l_ailp;
 	spin_lock(&ailp->xa_lock);
 	lip = xfs_trans_ail_cursor_first(ailp, &cur, 0);
+#ifdef DEBUG
 	last_lsn = xlog_assign_lsn(log->l_curr_cycle, log->l_curr_block);
+#endif
 	while (lip != NULL) {
 		/*
 		 * We're done when we see something other than an intent.

Dave Chinner

Reviewed-by on v2 1/1

2017-09-01 00:29 UTC #1 lore raw

Quoted 3 lines · Darrick J. Wong

Fix up all the compiler warnings that have crept in.
Signed-off-by Darrick J. Wong <[email protected]>

Looks good, warnings gone.

Reviewed-by Dave Chinner <[email protected]>

Cheers,

Dave.

Signature · 2 lines

Dave Chinner
[email protected]

Christoph Hellwig

2017-09-01 08:10 UTC #2 lore raw

In reply to Darrick J. Wong

Can we please just ensure ASSERT always evaluates the argument, similar to BUG()?

Darrick J. Wong

author

2017-09-01 15:55 UTC #3 lore raw

Quoted 2 lines · Christoph Hellwig

Can we please just ensure ASSERT always evaluates the argument, similar to BUG()?

Assuming you meant BUG_ON or something that actually takes a parameter, yes you can do that, though now we have to fix up a bunch of things like the xfs_trans debug counter asserts and anything that asserts on xfs_isilocked.

-.text                         785555      0
+.text                         787843      0

I guess that's only 0.29% larger...

--D

Quoted 4 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Brian Foster

2017-09-01 10:26 UTC #4 lore raw

In reply to Darrick J. Wong

Quoted 36 lines · Darrick J. Wong

Fix up all the compiler warnings that have crept in.

Signed-off-by Darrick J. Wong <[email protected]>

---
 fs/xfs/libxfs/xfs_inode_fork.c |    9 +++------
 fs/xfs/xfs_buf_item.c          |    2 ++
 fs/xfs/xfs_log_recover.c       |    4 ++++
 3 files changed, 9 insertions(+), 6 deletions(-)

fs/xfs/libxfs/xfs_inode_fork.c +3 -6

index fb310d0..31840ca 100644
@@ -1499,14 +1499,11 @@ xfs_iext_realloc_indirect(
 	xfs_ifork_t	*ifp,		/* inode fork pointer */
 	int		new_size)	/* new indirection array size */
 {
-	int		nlists;		/* number of irec's (ex lists) */
-	int		size;		/* current indirection array size */
-
 	ASSERT(ifp->if_flags & XFS_IFEXTIREC);
-	nlists = ifp->if_real_bytes / XFS_IEXT_BUFSZ;
-	size = nlists * sizeof(xfs_ext_irec_t);
 	ASSERT(ifp->if_real_bytes);
-	ASSERT((new_size >= 0) && (new_size != size));
+	ASSERT((new_size >= 0) &&
+	       (new_size != ((ifp->if_real_bytes / XFS_IEXT_BUFSZ) *
+			     sizeof(xfs_ext_irec_t))));
 	if (new_size == 0) {
 		xfs_iext_destroy(ifp);
 	} else {

fs/xfs/xfs_buf_item.c +0 -0

index ef2c137..1a7814a 100644

fs/xfs/xfs_buf_item.c xfs_buf_item_unlock( · line 576

Quoted 7 lines

fs/xfs/xfs_buf_item.c +2 -0

@@ -570,7 +570,9 @@ xfs_buf_item_unlock(
 	bool			aborted = !!(lip->li_flags & XFS_LI_ABORTED);
 	bool			hold = !!(bip->bli_flags & XFS_BLI_HOLD);
 	bool			dirty = !!(bip->bli_flags & XFS_BLI_DIRTY);
+#ifdef DEBUG
 	bool			ordered = !!(bip->bli_flags & XFS_BLI_ORDERED);
+#endif

This and the hunk below will break the CONFIG_XFS_WARN build.

Brian

Quoted 27 lines

fs/xfs/xfs_buf_item.c +0 -0

 	/* Clear the buffer's association with this transaction. */
 	bp->b_transp = NULL;

fs/xfs/xfs_log_recover.c +4 -1

index a362399..04a131c 100644
@@ -4827,12 +4827,16 @@ xlog_recover_process_intents(
 	int			error = 0;
 	struct xfs_ail_cursor	cur;
 	struct xfs_ail		*ailp;
+#ifdef DEBUG
 	xfs_lsn_t		last_lsn;
+#endif
 
 	ailp = log->l_ailp;
 	spin_lock(&ailp->xa_lock);
 	lip = xfs_trans_ail_cursor_first(ailp, &cur, 0);
+#ifdef DEBUG
 	last_lsn = xlog_assign_lsn(log->l_curr_cycle, log->l_curr_block);
+#endif
 	while (lip != NULL) {
 		/*
 		 * We're done when we see something other than an intent.
--

To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Darrick J. Wong

author

2017-09-01 16:19 UTC #5 lore raw

Quoted 38 lines · Brian Foster

Quoted 37 lines · Darrick J. Wong

Fix up all the compiler warnings that have crept in.

Signed-off-by Darrick J. Wong <[email protected]>

---
 fs/xfs/libxfs/xfs_inode_fork.c |    9 +++------
 fs/xfs/xfs_buf_item.c          |    2 ++
 fs/xfs/xfs_log_recover.c       |    4 ++++
 3 files changed, 9 insertions(+), 6 deletions(-)

fs/xfs/libxfs/xfs_inode_fork.c +3 -6

index fb310d0..31840ca 100644
@@ -1499,14 +1499,11 @@ xfs_iext_realloc_indirect(
 	xfs_ifork_t	*ifp,		/* inode fork pointer */
 	int		new_size)	/* new indirection array size */
 {
-	int		nlists;		/* number of irec's (ex lists) */
-	int		size;		/* current indirection array size */
-
 	ASSERT(ifp->if_flags & XFS_IFEXTIREC);
-	nlists = ifp->if_real_bytes / XFS_IEXT_BUFSZ;
-	size = nlists * sizeof(xfs_ext_irec_t);
 	ASSERT(ifp->if_real_bytes);
-	ASSERT((new_size >= 0) && (new_size != size));
+	ASSERT((new_size >= 0) &&
+	       (new_size != ((ifp->if_real_bytes / XFS_IEXT_BUFSZ) *
+			     sizeof(xfs_ext_irec_t))));
 	if (new_size == 0) {
 		xfs_iext_destroy(ifp);
 	} else {

fs/xfs/xfs_buf_item.c +0 -0

index ef2c137..1a7814a 100644
@@ -570,7 +570,9 @@ xfs_buf_item_unlock(
 	bool			aborted = !!(lip->li_flags & XFS_LI_ABORTED);

Quoted 8 lines

fs/xfs/xfs_buf_item.c xfs_buf_item_unlock( · line 576
Quoted 6 lines
fs/xfs/xfs_buf_item.c +2 -0
 	bool			hold = !!(bip->bli_flags & XFS_BLI_HOLD);
 	bool			dirty = !!(bip->bli_flags & XFS_BLI_DIRTY);
+#ifdef DEBUG
 	bool			ordered = !!(bip->bli_flags & XFS_BLI_ORDERED);
+#endif
 
This and the hunk below will break the CONFIG_XFS_WARN build.

Oops, good catch.

Quoted 16 lines

Brian

Quoted 14 lines

fs/xfs/xfs_buf_item.c +0 -0

 	/* Clear the buffer's association with this transaction. */
 	bp->b_transp = NULL;

fs/xfs/xfs_log_recover.c +2 -0

index a362399..04a131c 100644
@@ -4827,12 +4827,16 @@ xlog_recover_process_intents(
 	int			error = 0;
 	struct xfs_ail_cursor	cur;
 	struct xfs_ail		*ailp;
+#ifdef DEBUG
 	xfs_lsn_t		last_lsn;
+#endif

Quoted 6 lines

fs/xfs/xfs_log_recover.c xlog_recover_process_intents( · line 4839
Quoted 6 lines
fs/xfs/xfs_log_recover.c +2 -0
 	ailp = log->l_ailp;
 	spin_lock(&ailp->xa_lock);
 	lip = xfs_trans_ail_cursor_first(ailp, &cur, 0);
+#ifdef DEBUG
 	last_lsn = xlog_assign_lsn(log->l_curr_cycle, log->l_curr_block);
+#endif

This one ought to be fine, because the ASSERT is itself protected by an '#ifdef DEBUG'.

--D

Quoted 11 lines

Quoted 7 lines
fs/xfs/xfs_log_recover.c +0 -1
 	while (lip != NULL) {
 		/*
 		 * We're done when we see something other than an intent.
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected]
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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Brian Foster

2017-09-01 16:30 UTC #6 lore raw

Quoted 66 lines · Darrick J. Wong

Quoted 38 lines · Brian Foster

Quoted 37 lines · Darrick J. Wong

Fix up all the compiler warnings that have crept in.

Signed-off-by Darrick J. Wong <[email protected]>

---
 fs/xfs/libxfs/xfs_inode_fork.c |    9 +++------
 fs/xfs/xfs_buf_item.c          |    2 ++
 fs/xfs/xfs_log_recover.c       |    4 ++++
 3 files changed, 9 insertions(+), 6 deletions(-)

fs/xfs/libxfs/xfs_inode_fork.c +3 -6

index fb310d0..31840ca 100644
@@ -1499,14 +1499,11 @@ xfs_iext_realloc_indirect(
 	xfs_ifork_t	*ifp,		/* inode fork pointer */
 	int		new_size)	/* new indirection array size */
 {
-	int		nlists;		/* number of irec's (ex lists) */
-	int		size;		/* current indirection array size */
-
 	ASSERT(ifp->if_flags & XFS_IFEXTIREC);
-	nlists = ifp->if_real_bytes / XFS_IEXT_BUFSZ;
-	size = nlists * sizeof(xfs_ext_irec_t);
 	ASSERT(ifp->if_real_bytes);
-	ASSERT((new_size >= 0) && (new_size != size));
+	ASSERT((new_size >= 0) &&
+	       (new_size != ((ifp->if_real_bytes / XFS_IEXT_BUFSZ) *
+			     sizeof(xfs_ext_irec_t))));
 	if (new_size == 0) {
 		xfs_iext_destroy(ifp);
 	} else {

fs/xfs/xfs_buf_item.c +0 -0

index ef2c137..1a7814a 100644
@@ -570,7 +570,9 @@ xfs_buf_item_unlock(
 	bool			aborted = !!(lip->li_flags & XFS_LI_ABORTED);

Quoted 8 lines

fs/xfs/xfs_buf_item.c xfs_buf_item_unlock( · line 576
Quoted 6 lines
fs/xfs/xfs_buf_item.c +2 -0
 	bool			hold = !!(bip->bli_flags & XFS_BLI_HOLD);
 	bool			dirty = !!(bip->bli_flags & XFS_BLI_DIRTY);
+#ifdef DEBUG
 	bool			ordered = !!(bip->bli_flags & XFS_BLI_ORDERED);
+#endif
 
This and the hunk below will break the CONFIG_XFS_WARN build.

Oops, good catch.

Quoted 16 lines

Brian

Quoted 14 lines

fs/xfs/xfs_buf_item.c +0 -0

 	/* Clear the buffer's association with this transaction. */
 	bp->b_transp = NULL;

fs/xfs/xfs_log_recover.c +2 -0

index a362399..04a131c 100644
@@ -4827,12 +4827,16 @@ xlog_recover_process_intents(
 	int			error = 0;
 	struct xfs_ail_cursor	cur;
 	struct xfs_ail		*ailp;
+#ifdef DEBUG
 	xfs_lsn_t		last_lsn;
+#endif

Quoted 10 lines

Quoted 6 lines
fs/xfs/xfs_log_recover.c xlog_recover_process_intents( · line 4839
Quoted 6 lines
fs/xfs/xfs_log_recover.c +2 -0
 	ailp = log->l_ailp;
 	spin_lock(&ailp->xa_lock);
 	lip = xfs_trans_ail_cursor_first(ailp, &cur, 0);
+#ifdef DEBUG
 	last_lsn = xlog_assign_lsn(log->l_curr_cycle, log->l_curr_block);
+#endif
This one ought to be fine, because the ASSERT is itself protected by an '#ifdef DEBUG'.

That looks like a different assert from the one that uses last_lsn..?

Brian

Quoted 13 lines

--D
Quoted 11 lines
Quoted 7 lines
fs/xfs/xfs_log_recover.c +0 -1
 	while (lip != NULL) {
 		/*
 		 * We're done when we see something other than an intent.
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected]
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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Darrick J. Wong

author

2017-09-01 16:33 UTC #7 lore raw

Quoted 78 lines · Brian Foster

Quoted 66 lines · Darrick J. Wong

Quoted 38 lines · Brian Foster

Quoted 37 lines · Darrick J. Wong

Fix up all the compiler warnings that have crept in.

Signed-off-by Darrick J. Wong <[email protected]>

---
 fs/xfs/libxfs/xfs_inode_fork.c |    9 +++------
 fs/xfs/xfs_buf_item.c          |    2 ++
 fs/xfs/xfs_log_recover.c       |    4 ++++
 3 files changed, 9 insertions(+), 6 deletions(-)

fs/xfs/libxfs/xfs_inode_fork.c +3 -6

index fb310d0..31840ca 100644
@@ -1499,14 +1499,11 @@ xfs_iext_realloc_indirect(
 	xfs_ifork_t	*ifp,		/* inode fork pointer */
 	int		new_size)	/* new indirection array size */
 {
-	int		nlists;		/* number of irec's (ex lists) */
-	int		size;		/* current indirection array size */
-
 	ASSERT(ifp->if_flags & XFS_IFEXTIREC);
-	nlists = ifp->if_real_bytes / XFS_IEXT_BUFSZ;
-	size = nlists * sizeof(xfs_ext_irec_t);
 	ASSERT(ifp->if_real_bytes);
-	ASSERT((new_size >= 0) && (new_size != size));
+	ASSERT((new_size >= 0) &&
+	       (new_size != ((ifp->if_real_bytes / XFS_IEXT_BUFSZ) *
+			     sizeof(xfs_ext_irec_t))));
 	if (new_size == 0) {
 		xfs_iext_destroy(ifp);
 	} else {

fs/xfs/xfs_buf_item.c +0 -0

index ef2c137..1a7814a 100644
@@ -570,7 +570,9 @@ xfs_buf_item_unlock(
 	bool			aborted = !!(lip->li_flags & XFS_LI_ABORTED);

Quoted 8 lines

fs/xfs/xfs_buf_item.c xfs_buf_item_unlock( · line 576
Quoted 6 lines
fs/xfs/xfs_buf_item.c +2 -0
 	bool			hold = !!(bip->bli_flags & XFS_BLI_HOLD);
 	bool			dirty = !!(bip->bli_flags & XFS_BLI_DIRTY);
+#ifdef DEBUG
 	bool			ordered = !!(bip->bli_flags & XFS_BLI_ORDERED);
+#endif
 
This and the hunk below will break the CONFIG_XFS_WARN build.

Oops, good catch.

Quoted 16 lines

Brian

Quoted 14 lines

fs/xfs/xfs_buf_item.c +0 -0

 	/* Clear the buffer's association with this transaction. */
 	bp->b_transp = NULL;

fs/xfs/xfs_log_recover.c +2 -0

index a362399..04a131c 100644
@@ -4827,12 +4827,16 @@ xlog_recover_process_intents(
 	int			error = 0;
 	struct xfs_ail_cursor	cur;
 	struct xfs_ail		*ailp;
+#ifdef DEBUG
 	xfs_lsn_t		last_lsn;
+#endif

Quoted 10 lines

Quoted 6 lines
fs/xfs/xfs_log_recover.c xlog_recover_process_intents( · line 4839
Quoted 6 lines
fs/xfs/xfs_log_recover.c +2 -0
 	ailp = log->l_ailp;
 	spin_lock(&ailp->xa_lock);
 	lip = xfs_trans_ail_cursor_first(ailp, &cur, 0);
+#ifdef DEBUG
 	last_lsn = xlog_assign_lsn(log->l_curr_cycle, log->l_curr_block);
+#endif
This one ought to be fine, because the ASSERT is itself protected by an '#ifdef DEBUG'.

Quoted 1 line

That looks like a different assert from the one that uses last_lsn..?

Doh, you're right.

--D

Quoted 20 lines

Brian
Quoted 13 lines
--D
Quoted 11 lines
Quoted 7 lines
fs/xfs/xfs_log_recover.c +0 -1
 	while (lip != NULL) {
 		/*
 		 * We're done when we see something other than an intent.
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected]
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 [email protected]
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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html