[PATCH] xfs: cleanup duplicate initializations

[PATCH] xfs: cleanup duplicate initializations

[David Sterba]

follow these guidelines:
- leave initialization in the declaration block if it fits the line
- move to the code where it's more suitable ('for' init block)

Signed-off-by: David Sterba <dsterba@suse.cz>
---
 fs/xfs/xfs_dfrag.c      |    6 +-----
 fs/xfs/xfs_inode_item.c |    1 -
 fs/xfs/xfs_mount.c      |    2 +-
 3 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/fs/xfs/xfs_dfrag.c b/fs/xfs/xfs_dfrag.c
index be62867..9a84a85 100644
--- a/fs/xfs/xfs_dfrag.c
+++ b/fs/xfs/xfs_dfrag.c
@@ -202,7 +202,7 @@ xfs_swap_extents(
 	xfs_inode_t	*tip,	/* tmp inode */
 	xfs_swapext_t	*sxp)
 {
-	xfs_mount_t	*mp;
+	xfs_mount_t	*mp = ip->i_mount;
 	xfs_trans_t	*tp;
 	xfs_bstat_t	*sbp = &sxp->sx_stat;
 	xfs_ifork_t	*tempifp, *ifp, *tifp;
@@ -212,16 +212,12 @@ xfs_swap_extents(
 	int		taforkblks = 0;
 	__uint64_t	tmp;
 
-	mp = ip->i_mount;
-
 	tempifp = kmem_alloc(sizeof(xfs_ifork_t), KM_MAYFAIL);
 	if (!tempifp) {
 		error = XFS_ERROR(ENOMEM);
 		goto out;
 	}
 
-	sbp = &sxp->sx_stat;
-
 	/*
 	 * we have to do two separate lock calls here to keep lockdep
 	 * happy. If we try to get all the locks in one call, lock will
diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
index 46cc401..c64458b 100644
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -957,7 +957,6 @@ xfs_iflush_abort(
 {
 	xfs_inode_log_item_t	*iip = ip->i_itemp;
 
-	iip = ip->i_itemp;
 	if (iip) {
 		struct xfs_ail	*ailp = iip->ili_item.li_ailp;
 		if (iip->ili_item.li_flags & XFS_LI_IN_AIL) {
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index bb3f9a7..08de13d 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -1900,7 +1900,7 @@ xfs_mod_incore_sb_batch(
 	uint			nmsb,
 	int			rsvd)
 {
-	xfs_mod_sb_t		*msbp = &msb[0];
+	xfs_mod_sb_t		*msbp;
 	int			error = 0;
 
 	/*
-- 
1.7.4.1.176.g501cc

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs: cleanup duplicate initializations

[Alex Elder]

On Mon, 2011-03-28 at 19:12 +0200, David Sterba wrote:
> follow these guidelines:
> - leave initialization in the declaration block if it fits the line
> - move to the code where it's more suitable ('for' init block)
> 
> Signed-off-by: David Sterba <dsterba@suse.cz>

Looks good.

Reviewed-by: Alex Elder <aelder@sgi.com>



_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH, v2] xfs: cleanup duplicate initializations

[Alex Elder]

On Mon, 2011-03-28 at 13:52 -0500, Alex Elder wrote:
> On Mon, 2011-03-28 at 19:12 +0200, David Sterba wrote:
> > follow these guidelines:
> > - leave initialization in the declaration block if it fits the line
> > - move to the code where it's more suitable ('for' init block)
> > 
> > Signed-off-by: David Sterba <dsterba@suse.cz>
> 
> Looks good.
> 
> Reviewed-by: Alex Elder <aelder@sgi.com>

I retract this.  The last chunk in the patch is erroneous.

Below is the original proposed patch, except I have
modified the last chunk to be a correct fix for what
appears to be a duplicate initialization.  (It was not,
really, but I've changed it so only one assignment is
made, and the result makes it more obvious what's
going on.)

David, perhaps you could sign off on this version.
Meanwhile, another reviewer might make sense.

					-Alex

follow these guidelines:
- leave initialization in the declaration block if it fits the line
- move to the code where it's more suitable ('for' init block)

Originally proposed by David Sterba <dsterba@suse.cz>

Signed-off-by: Alex Elder <aelder@sgi.com>

---
 fs/xfs/xfs_dfrag.c      |    6 +-----
 fs/xfs/xfs_inode_item.c |    1 -
 fs/xfs/xfs_mount.c      |    4 ++--
 3 files changed, 3 insertions(+), 8 deletions(-)

Index: b/fs/xfs/xfs_dfrag.c
===================================================================
--- a/fs/xfs/xfs_dfrag.c
+++ b/fs/xfs/xfs_dfrag.c
@@ -202,7 +202,7 @@ xfs_swap_extents(
        xfs_inode_t     *tip,   /* tmp inode */
        xfs_swapext_t   *sxp)
 {
-       xfs_mount_t     *mp;
+       xfs_mount_t     *mp = ip->i_mount;
        xfs_trans_t     *tp;
        xfs_bstat_t     *sbp = &sxp->sx_stat;
        xfs_ifork_t     *tempifp, *ifp, *tifp;
@@ -212,16 +212,12 @@ xfs_swap_extents(
        int             taforkblks = 0;
        __uint64_t      tmp;
 
-       mp = ip->i_mount;
-
        tempifp = kmem_alloc(sizeof(xfs_ifork_t), KM_MAYFAIL);
        if (!tempifp) {
                error = XFS_ERROR(ENOMEM);
                goto out;
        }
 
-       sbp = &sxp->sx_stat;
-
        /*
         * we have to do two separate lock calls here to keep lockdep
         * happy. If we try to get all the locks in one call, lock will
Index: b/fs/xfs/xfs_inode_item.c
===================================================================
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -970,7 +970,6 @@ xfs_iflush_abort(
 {
        xfs_inode_log_item_t    *iip = ip->i_itemp;
 
-       iip = ip->i_itemp;
        if (iip) {
                struct xfs_ail  *ailp = iip->ili_item.li_ailp;
                if (iip->ili_item.li_flags & XFS_LI_IN_AIL) {
Index: b/fs/xfs/xfs_mount.c
===================================================================
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -1900,7 +1900,7 @@ xfs_mod_incore_sb_batch(
        uint                    nmsb,
        int                     rsvd)
 {
-       xfs_mod_sb_t            *msbp = &msb[0];
+       xfs_mod_sb_t            *msbp;
        int                     error = 0;
 
        /*
@@ -1910,7 +1910,7 @@ xfs_mod_incore_sb_batch(
         * changes will be atomic.
         */
        spin_lock(&mp->m_sb_lock);
-       for (msbp = &msbp[0]; msbp < (msb + nmsb); msbp++) {
+       for (msbp = msb; msbp < (msb + nmsb); msbp++) {
                ASSERT(msbp->msb_field < XFS_SBS_ICOUNT ||
                       msbp->msb_field > XFS_SBS_FDBLOCKS);
 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH, v2] xfs: cleanup duplicate initializations

[Dave Chinner]

On Wed, Apr 13, 2011 at 05:03:28PM -0500, Alex Elder wrote:
> On Mon, 2011-03-28 at 13:52 -0500, Alex Elder wrote:
> > On Mon, 2011-03-28 at 19:12 +0200, David Sterba wrote:
> > > follow these guidelines:
> > > - leave initialization in the declaration block if it fits the line
> > > - move to the code where it's more suitable ('for' init block)
> > > 
> > > Signed-off-by: David Sterba <dsterba@suse.cz>
> > 
> > Looks good.
> > 
> > Reviewed-by: Alex Elder <aelder@sgi.com>
> 
> I retract this.  The last chunk in the patch is erroneous.
> 
> Below is the original proposed patch, except I have
> modified the last chunk to be a correct fix for what
> appears to be a duplicate initialization.  (It was not,
> really, but I've changed it so only one assignment is
> made, and the result makes it more obvious what's
> going on.)
> 
> David, perhaps you could sign off on this version.
> Meanwhile, another reviewer might make sense.

Seeing as you only added a hunk, I'd say that keeping his old
sіgnoff is just fine.

> 
> 					-Alex
> 
> follow these guidelines:
> - leave initialization in the declaration block if it fits the line
> - move to the code where it's more suitable ('for' init block)
> 
> Originally proposed by David Sterba <dsterba@suse.cz>

That is what the "From:" tag is for when you post someone else's
patch. ;)

Anyway, looks good now.

Reviewed-by: Dave Chinner <dchinner@redhat.com>

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH, v2] xfs: cleanup duplicate initializations

[Alex Elder]

On Thu, 2011-04-14 at 08:53 +1000, Dave Chinner wrote:
> On Wed, Apr 13, 2011 at 05:03:28PM -0500, Alex Elder wrote:
. . .
> > David, perhaps you could sign off on this version.
> > Meanwhile, another reviewer might make sense.
> 
> Seeing as you only added a hunk, I'd say that keeping his old
> sіgnoff is just fine.

OK.

> > 
> > 					-Alex
> > 
> > follow these guidelines:
> > - leave initialization in the declaration block if it fits the line
> > - move to the code where it's more suitable ('for' init block)
> > 
> > Originally proposed by David Sterba <dsterba@suse.cz>
> 
> That is what the "From:" tag is for when you post someone else's
> patch. ;)

Good to know.  Thanks a lot for the review.

> Anyway, looks good now.
> 
> Reviewed-by: Dave Chinner <dchinner@redhat.com>
> 
> Cheers,
> 
> Dave.



_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs