On Thu, Jan 03, 2008 at 12:37:12PM +0100, rubisher wrote:
> Hello *,
>
> I am not sure to have perfectly understand all subtle details of likely() &
> unlikely() macros but I think there's some brace at the bad place in following
> chunk:
> --- arch/parisc/lib/memcpy.c.Orig 2007-10-18 15:27:30.000000000 +0000
> +++ arch/parisc/lib/memcpy.c 2008-01-03 10:17:52.000000000 +0000
> @@ -299,7 +299,7 @@
>
> /* Check alignment */
> t1 = (src ^ dst);
> - if (unlikely(t1 & (sizeof(double)-1)))
> + if (unlikely(t1 & (sizeof(double) - 1)))
Please submit separate patches for the white space clean up.
> goto unaligned_copy;
>
> /* src and dst have same alignment. */
> @@ -405,7 +405,7 @@
>
> unaligned_copy:
> /* possibly we are aligned on a word, but not on a double... */
> - if (likely(t1 & (sizeof(unsigned int)-1)) == 0) {
> + if (likely((t1 & (sizeof(unsigned int) - 1)) == 0)) {
> t2 = src & (sizeof(unsigned int) - 1);
>
> if (unlikely(t2 != 0)) {
> === <> ===
> First hunk is just to add some whitespace?
>
> Otoh for the second hunk, my reading of the original stuff was that:
> > likely(t1 & (sizeof(unsigned int)-1))
> i.e. likely's macro embraced only "t1 & (sizeof(unsigned int)-1)" in place of
> "(t1 & (sizeof(unsigned int)-1)) == 0".
>
> What's your opinion?
I think you are right. normally likely() and unlikely() are intended
to be used with boolean expressions and that's obviously not the case.
Please resubmit this change separately and add "signed-off-by" lines
so kyle can include this in the next round of parisc patches.
cheers,
grant
>
> Tia,
> r.
> ---
> Scarlet One, ADSL 6 Mbps + Telephone, from EUR 29,95...
> http://www.scarlet.be/
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
-
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
