From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755447AbdJIWa7 (ORCPT ); Mon, 9 Oct 2017 18:30:59 -0400 Received: from mail.kernel.org ([198.145.29.99]:50414 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754404AbdJIWa5 (ORCPT ); Mon, 9 Oct 2017 18:30:57 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 41BBA218BB Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=goodmis.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=rostedt@goodmis.org Date: Mon, 9 Oct 2017 18:30:55 -0400 From: Steven Rostedt To: Michael Sartain Cc: linux-kernel@vger.kernel.org Subject: Re: [PATCH 4/5] trace-cmd: Use unsigned values in Hsieh's trace_hash fast hash function Message-ID: <20171009183055.567190e7@gandalf.local.home> In-Reply-To: <20170812173047.840-5-mikesart@fastmail.com> References: <20170812173047.840-1-mikesart@fastmail.com> <20170812173047.840-5-mikesart@fastmail.com> X-Mailer: Claws Mail 3.14.0 (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, 12 Aug 2017 11:30:46 -0600 Michael Sartain wrote: > Signed int values were being used where the original code used uint32_t types: > > http://www.azillionmonkeys.com/qed/hash.html > > Right shifting negative int values has implementation-defined and left shifting > has undefined behavior. > > On my platform (x86_64) right shifting was doing sign extension and filling > high bits with 1s, which is different than the original algorithm. > Heh, nice catch. Although the hash was never used for anything too important. Mostly just colors of the graph. > Signed-off-by: Michael Sartain > --- > trace-hash-local.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/trace-hash-local.h b/trace-hash-local.h > index b2a1002..b3f9b06 100644 > --- a/trace-hash-local.h > +++ b/trace-hash-local.h > @@ -22,7 +22,7 @@ > > static inline unsigned int trace_hash(int val) > { > - int hash, tmp; > + unsigned int hash, tmp; > > hash = 12546869; /* random prime */ > > @@ -34,7 +34,7 @@ static inline unsigned int trace_hash(int val) > */ > > hash += (val & 0xffff); > - tmp = (val >> 16) ^ hash; > + tmp = ((unsigned int)val >> 16) ^ hash; Why not just have val be unsigned int, and not do the typecast? -- Steve > hash = (hash << 16) ^ tmp; > hash += hash >> 11; >