public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Michal Januszewski <michalj@gmail.com>
To: linux-kernel@vger.kernel.org
Subject: [PATCH] Make sure abs() always works on a signed argument
Date: Wed, 28 Jul 2010 14:44:25 +0200	[thread overview]
Message-ID: <20100728123048.GA4127@quadria> (raw)

The abs() macro is commonly used to calculate the modulus of a difference
of two numbers.  With the introduction of "long" inside the definition
of abs() in the commit a49c59c042c63b432307c1bbf7dac5a104c786e6, some of
the abs() calls in the kernel started returning unexpected values (for
instance, see abs() usage in drivers/video/modedb.c and the resulting
problems reported in https://bugs.gentoo.org/show_bug.cgi?id=296539).

The problem is apparent if the argument of abs() is a difference of two
32-bit integers, at least one of which is unsigned.  The result is then
assumed to be an unsigned integer, which gets cast to a positive long. The
return value of abs() is then this large positive integer, instead of the
expected small positive integer representing the modulus of the argument.

Example:

  u32 a = 0, b = 1;
  u32 c = abs(a - b);

  'c' will end up with a value of 0xffffffff instead of the expected 0x1.

To fix this problem, explicitly cast the argument of abs() to signed, so
that it gets properly expanded to long in case the original argument was
an int.

Signed-off-by: Michal Januszewski <michalj@gmail.com>
--
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 8317ec4..29fd43e 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -158,7 +158,7 @@ extern int _cond_resched(void);
 #define might_sleep_if(cond) do { if (cond) might_sleep(); } while (0)
 
 #define abs(x) ({				\
-		long __x = (x);			\
+		long __x = (signed)(x);		\
 		(__x < 0) ? -__x : __x;		\
 	})
 

             reply	other threads:[~2010-07-28 12:44 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-07-28 12:44 Michal Januszewski [this message]
2010-07-28 13:44 ` [PATCH] Make sure abs() always works on a signed argument Andreas Schwab
2010-07-28 15:28   ` [PATCH] Make sure abs() works as expected with both ints and longs Michal Januszewski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100728123048.GA4127@quadria \
    --to=michalj@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox