linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH 1/1] fs: Small refactoring of the code in ext4 2.6.37-rc1
@ 2010-11-04 23:34 André Luis Pereira dos Santos - BSRSoft
  2010-11-05 12:52 ` Ted Ts'o
  0 siblings, 1 reply; 5+ messages in thread
From: André Luis Pereira dos Santos - BSRSoft @ 2010-11-04 23:34 UTC (permalink / raw)
  To: Andreas Dilger; +Cc: linux-ext4, linux-kernel

  Hello.

This is true.
Despite having gone through all compilation and I have not noticed apparent problems, my mistake is evident in this case.

I had not realized that the side effect would really be changing variables incremented and decremented.

Thank you.

I'll think more about this type of side effect. :)


On Thu, 4 Nov 2010, André Luis Pereira dos Santos - BSRSoft wrote:

> From: Andre Luis Pereira dos Santos <andre@bsrsoft.com.br>
>
> Hi.
> Small refactoring of the code in order to make minor enhancements to critical areas.
> The notation x + 1 has been replaced by more efficient notation x + +.
>
> Signed-off-by: Andre Luis Pereira dos Santos <andre@bsrsoft.com.br>
> ---
> Signed-off-by: Andre Luis Pereira dos Santos <andre@bsrsoft.com.br>
> --- linux-2.6.37-rc1/fs/ext4/extents.c        2010-11-01 09:54:12.000000000 -0200
> +++ linux-2.6.37-rc1-patched/fs/ext4/extents.c        2010-11-04 19:54:26.000000000 -0200
> @@ -555,9 +555,9 @@ ext4_ext_binsearch(struct inode *inode,
>       while (l <= r) {
>               m = l + (r - l) / 2;
>               if (block < le32_to_cpu(m->ee_block))
> -                     r = m - 1;
> +                     r = m--;
>               else
> -                     l = m + 1;
> +                     l = m++;

These do not give identical results.

foo = bar + 1;  assigns (bar + 1) to foo.
foo = bar--;  assigns bar to foo then decrements bar.
foo = --bar;  decrements bar then assigns bar to foo.

So your change both change the value that will be assigned to 'r' and 'l'
and also modify 'm' which was not previously modified.


>               ext_debug("%p(%u):%p(%u):%p(%u) ", l, le32_to_cpu(l->ee_block),
>                               m, le32_to_cpu(m->ee_block),
>                               r, le32_to_cpu(r->ee_block));
> @@ -1557,7 +1557,7 @@ static int ext4_ext_try_to_merge(struct
>               if (ext4_ext_is_uninitialized(ex))
>                       uninitialized = 1;
>               ex->ee_len = cpu_to_le16(ext4_ext_get_actual_len(ex)
> -                             + ext4_ext_get_actual_len(ex + 1));
> +                             + ext4_ext_get_actual_len(ex++));

After your change gcc complains:

 fs/ext4/extents.c:1559:16: warning: operation on ‘ex’ may be undefined
 fs/ext4/extents.c:1559:16: warning: operation on ‘ex’ may be undefined

which it is correct in doing since you are now modifying the value of the
pointer which is dereferenced in the assignment. Previously the value of
(ex+1) was simply passed to ext4_ext_get_actual_len(), but now you are
passing the value of (ex) to ext4_ext_get_actual_len() and then
subsequently incrementing 'ex' itself.


>               if (uninitialized)
>                       ext4_ext_mark_uninitialized(ex);
>
>


Was this patch even compile tested?


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

^ permalink raw reply	[flat|nested] 5+ messages in thread
* [PATCH 1/1] fs: Small refactoring of the code in ext4 2.6.37-rc1
@ 2010-11-04 22:07 André Luis Pereira dos Santos - BSRSoft
  2010-11-04 22:31 ` Jesper Juhl
  2010-11-04 22:44 ` Greg Freemyer
  0 siblings, 2 replies; 5+ messages in thread
From: André Luis Pereira dos Santos - BSRSoft @ 2010-11-04 22:07 UTC (permalink / raw)
  To: Andreas Dilger; +Cc: linux-ext4, linux-kernel

From: Andre Luis Pereira dos Santos <andre@bsrsoft.com.br>

Hi.
Small refactoring of the code in order to make minor enhancements to critical areas.
The notation x + 1 has been replaced by more efficient notation x + +.

Signed-off-by: Andre Luis Pereira dos Santos <andre@bsrsoft.com.br>
---
Signed-off-by: Andre Luis Pereira dos Santos <andre@bsrsoft.com.br>
--- linux-2.6.37-rc1/fs/ext4/extents.c	2010-11-01 09:54:12.000000000 -0200
+++ linux-2.6.37-rc1-patched/fs/ext4/extents.c	2010-11-04 19:54:26.000000000 -0200
@@ -555,9 +555,9 @@ ext4_ext_binsearch(struct inode *inode,
 	while (l <= r) {
 		m = l + (r - l) / 2;
 		if (block < le32_to_cpu(m->ee_block))
-			r = m - 1;
+			r = m--;
 		else
-			l = m + 1;
+			l = m++;
 		ext_debug("%p(%u):%p(%u):%p(%u) ", l, le32_to_cpu(l->ee_block),
 				m, le32_to_cpu(m->ee_block),
 				r, le32_to_cpu(r->ee_block));
@@ -1557,7 +1557,7 @@ static int ext4_ext_try_to_merge(struct
 		if (ext4_ext_is_uninitialized(ex))
 			uninitialized = 1;
 		ex->ee_len = cpu_to_le16(ext4_ext_get_actual_len(ex)
-				+ ext4_ext_get_actual_len(ex + 1));
+				+ ext4_ext_get_actual_len(ex++));
 		if (uninitialized)
 			ext4_ext_mark_uninitialized(ex);


      

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-11-05 12:52 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-11-04 23:34 [PATCH 1/1] fs: Small refactoring of the code in ext4 2.6.37-rc1 André Luis Pereira dos Santos - BSRSoft
2010-11-05 12:52 ` Ted Ts'o
  -- strict thread matches above, loose matches on Subject: below --
2010-11-04 22:07 André Luis Pereira dos Santos - BSRSoft
2010-11-04 22:31 ` Jesper Juhl
2010-11-04 22:44 ` Greg Freemyer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).