Use kernel.h macro definition.
Also remove assignment in if condition (checkpatch error)
Signed-off-by: Fabian Frederick <[email protected]>
---
fs/xfs/libxfs/xfs_dir2_node.c | 8 ++------
Signed-off-by: Fabian Frederick <[email protected]>
---
fs/xfs/libxfs/xfs_dir2_node.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_dir2_node.c b/fs/xfs/libxfs/xfs_dir2_node.c
index 41b80d3..84442a6 100644
--- a/fs/xfs/libxfs/xfs_dir2_node.c
+++ b/fs/xfs/libxfs/xfs_dir2_node.c
@@ -967,13 +967,10 @@ xfs_dir2_leafn_rebalance(
/*
* If the block order is wrong, swap the arguments.
*/
- if ((swap = xfs_dir2_leafn_order(dp, blk1->bp, blk2->bp))) {
- xfs_da_state_blk_t *tmp; /* temp for block swap */
+ swap = xfs_dir2_leafn_order(dp, blk1->bp, blk2->bp);
+ if (swap)
+ swap(blk1, blk2);
- tmp = blk1;
- blk1 = blk2;
- blk2 = tmp;
- }
leaf1 = blk1->bp->b_addr;
leaf2 = blk2->bp->b_addr;
dp->d_ops->leaf_hdr_from_disk(&hdr1, leaf1);
--
2.4.0
Use kernel.h macro definition.
Signed-off-by: Fabian Frederick <[email protected]>
---
fs/xfs/xfs_inode.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index d6ebc85..d4f3a09 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -500,7 +500,6 @@ xfs_lock_two_inodes(
xfs_inode_t *ip1,
uint lock_mode)
{
- xfs_inode_t *temp;
int attempts = 0;
xfs_log_item_t *lp;
@@ -512,11 +511,8 @@ xfs_lock_two_inodes(
ASSERT(ip0->i_ino != ip1->i_ino);
- if (ip0->i_ino > ip1->i_ino) {
- temp = ip0;
- ip0 = ip1;
- ip1 = temp;
- }
+ if (ip0->i_ino > ip1->i_ino)
+ swap(ip0, ip1);
again:
xfs_ilock(ip0, xfs_lock_inumorder(lock_mode, 0));
--
2.4.0
Use kernel.h macro definition.
Signed-off-by: Fabian Frederick <[email protected]>
---
fs/xfs/libxfs/xfs_attr_leaf.c | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index 04e79d5..a346455 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -1481,17 +1481,10 @@ xfs_attr3_leaf_rebalance(
*/
swap = 0;
if (xfs_attr3_leaf_order(blk1->bp, &ichdr1, blk2->bp, &ichdr2)) {
- struct xfs_da_state_blk *tmp_blk;
- struct xfs_attr3_icleaf_hdr tmp_ichdr;
-
- tmp_blk = blk1;
- blk1 = blk2;
- blk2 = tmp_blk;
+ swap(blk1, blk2);
/* struct copies to swap them rather than reconverting */
- tmp_ichdr = ichdr1;
- ichdr1 = ichdr2;
- ichdr2 = tmp_ichdr;
+ swap(ichdr1, ichdr2);
leaf1 = blk1->bp->b_addr;
leaf2 = blk2->bp->b_addr;
--
2.4.0
Use kernel.h macro definition.
Signed-off-by: Fabian Frederick <[email protected]>
---
fs/xfs/libxfs/xfs_da_btree.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
index 2385f8c..f0101a8 100644
--- a/fs/xfs/libxfs/xfs_da_btree.c
+++ b/fs/xfs/libxfs/xfs_da_btree.c
@@ -734,7 +734,6 @@ xfs_da3_node_rebalance(
{
struct xfs_da_intnode *node1;
struct xfs_da_intnode *node2;
- struct xfs_da_intnode *tmpnode;
struct xfs_da_node_entry *btree1;
struct xfs_da_node_entry *btree2;
struct xfs_da_node_entry *btree_s;
@@ -764,9 +763,7 @@ xfs_da3_node_rebalance(
((be32_to_cpu(btree2[0].hashval) < be32_to_cpu(btree1[0].hashval)) ||
(be32_to_cpu(btree2[nodehdr2.count - 1].hashval) <
be32_to_cpu(btree1[nodehdr1.count - 1].hashval)))) {
- tmpnode = node1;
- node1 = node2;
- node2 = tmpnode;
+ swap(node1, node2);
dp->d_ops->node_hdr_from_disk(&nodehdr1, node1);
dp->d_ops->node_hdr_from_disk(&nodehdr2, node2);
btree1 = dp->d_ops->node_tree_p(node1);
--
2.4.0
On Mon, May 18, 2015 at 07:13:48PM +0200, Fabian Frederick wrote:
> * If the block order is wrong, swap the arguments.
> */
> - if ((swap = xfs_dir2_leafn_order(dp, blk1->bp, blk2->bp))) {
> - xfs_da_state_blk_t *tmp; /* temp for block swap */
> + swap = xfs_dir2_leafn_order(dp, blk1->bp, blk2->bp);
> + if (swap)
> + swap(blk1, blk2);
Egads... Have you even read what you'd written? Yes, sure, preprocessor
will do the right thing, but it's a very noticable annoyance for somebody
reading that. Rename the bleeding flag, please.
> On 18 May 2015 at 19:19 Al Viro <[email protected]> wrote:
>
>
> On Mon, May 18, 2015 at 07:13:48PM +0200, Fabian Frederick wrote:
> > * If the block order is wrong, swap the arguments.
> > */
> > - if ((swap = xfs_dir2_leafn_order(dp, blk1->bp, blk2->bp))) {
> > - xfs_da_state_blk_t *tmp; /* temp for block swap */
> > + swap = xfs_dir2_leafn_order(dp, blk1->bp, blk2->bp);
> > + if (swap)
> > + swap(blk1, blk2);
>
> Egads... Have you even read what you'd written? Yes, sure, preprocessor
> will do the right thing, but it's a very noticable annoyance for somebody
> reading that. Rename the bleeding flag, please.
I wanted to focus on the swap() update in this small patchset (some other things
should be done in there like have xfs_dir2_leafn_order() return bool) but I can
rename it in something like need_swap. Do I need to resend the 4 patches Dave ?
Regards,
Fabian
On Mon, May 18, 2015 at 08:06:13PM +0200, Fabian Frederick wrote:
>
>
> > On 18 May 2015 at 19:19 Al Viro <[email protected]> wrote:
> >
> >
> > On Mon, May 18, 2015 at 07:13:48PM +0200, Fabian Frederick wrote:
> > >? ? ? * If the block order is wrong, swap the arguments.
> > >? ? ? */
> > > -? ?if ((swap = xfs_dir2_leafn_order(dp, blk1->bp, blk2->bp))) {
> > > -? ? ? ? ? ?xfs_da_state_blk_t? ? ? *tmp;? ?/* temp for block swap */
> > > +? ?swap = xfs_dir2_leafn_order(dp, blk1->bp, blk2->bp);
> > > +? ?if (swap)
> > > +? ? ? ? ? ?swap(blk1, blk2);
> >
> > Egads...? Have you even read what you'd written?? Yes, sure, preprocessor
> > will do the right thing, but it's a very noticable annoyance for somebody
> > reading that.? Rename the bleeding flag, please.
>
> I wanted to focus on the swap() update in this small patchset (some other things
> should be done in there like have xfs_dir2_leafn_order() return bool) but I can
> rename it in something like need_swap. Do I need to resend the 4 patches Dave ?
4 patches is 3 patches too many for noise like this. Anyway, two of
the patches have the same local "swap" variable problem; the context
is "swap order" not "need swap".
FWIW, I am not a fan of changing the code for no actual gain - the
compiled code is identical, there are no stack savings, and now I
have to look at an extra file to work out what the code does. If
you're changing the code and this is prep work for a large series,
then by all means clean the code up. But otherwise, changes like
this just mean work that other developers have in progress need
rebasing....
Cheers,
Dave.
--
Dave Chinner
[email protected]