mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Peter Firefly Lund <firefly@vax64.dk>
To: Christoph Lameter <clameter@sgi.com>
Cc: Momchil Velikov <velco@fadata.bg>,
	Christoph Hellwig <hch@infradead.org>,
	linux-kernel@vger.kernel.org, trivial@kernel.org
Subject: [PATCH] avoid negative shifts in radix-tree.c
Date: Sat, 25 Aug 2007 16:26:07 +0200	[thread overview]
Message-ID: <1188051967.18306.45.camel@ls-search> (raw)

(linux-vax@pergamentum.com only bcc'ed because it's subscribers only,
Lameter addressed because I think he touched the code last, Velikov and
Hellwig because they touched the code first.)

The current code in __max_index() will shift by a negative amount first
and only then fix the situation by ignoring the result if the shift
amount would have been negative.  This happens to work on almost any
architecture despite not being valid C.

Chapter and verse from the (draft) ISO C99 standard, "6.5.7 Bitwise
shift operators", paragraph 3:

  "The integer promotions are performed on each of the operands. The
   type of the result is that of the promoted left operand. If the
   value of the right operand is negative or is greater than or equal
   to the width of the promoted left operand, the behavior is
   undefined."

Right-shifting by a negative amount causes a "reserved operand fault" on
the VAX with some gcc versions.


Applies to 2.6.22.
Boot tested lightly on an emulated VAX.

(The function is called 7 times on booth with the values 0..6 -- I
checked that the return values were the same + that it returns ~0UL for
height==6 as was clearly the intention.)

-Peter

--- lib/radix-tree-old.c	2007-08-25 15:36:40.000000000 +0200
+++ lib/radix-tree.c	2007-08-25 15:36:51.000000000 +0200
@@ -980,12 +980,14 @@ radix_tree_node_ctor(void *node, struct 
 
 static __init unsigned long __maxindex(unsigned int height)
 {
-	unsigned int tmp = height * RADIX_TREE_MAP_SHIFT;
-	unsigned long index = (~0UL >> (RADIX_TREE_INDEX_BITS - tmp - 1)) >> 1;
-
-	if (tmp >= RADIX_TREE_INDEX_BITS)
-		index = ~0UL;
-	return index;
+	unsigned int	tmp   = height * RADIX_TREE_MAP_SHIFT;
+	int		shift = RADIX_TREE_INDEX_BITS - tmp;
+	unsigned long	index;
+
+	if (shift < 0)
+		return ~0UL;
+	else
+		return ~0UL >> shift;
 }
 
 static __init void radix_tree_init_maxindex(void)



             reply	other threads:[~2007-08-25 14:48 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-08-25 14:26 Peter Firefly Lund [this message]
2007-08-26 16:01 ` [LV] " Jan-Benedict Glaw
2007-08-27 13:28 ` Christoph Hellwig
2007-08-29 11:39   ` Maciej W. Rozycki

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=1188051967.18306.45.camel@ls-search \
    --to=firefly@vax64.dk \
    --cc=clameter@sgi.com \
    --cc=hch@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=trivial@kernel.org \
    --cc=velco@fadata.bg \
    /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

all inboxes | Powered by JetHome®