From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1751799AbbCTV2I (ORCPT <rfc822;w@1wt.eu>);
	Fri, 20 Mar 2015 17:28:08 -0400
Received: from mail.linuxfoundation.org ([140.211.169.12]:53572 "EHLO
	mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1751150AbbCTV2E (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Fri, 20 Mar 2015 17:28:04 -0400
Date: Fri, 20 Mar 2015 14:28:03 -0700
From: Andrew Morton <akpm@linux-foundation.org>
To: Liu Sha <rednoax@qq.com>
Cc: linux-kernel@vger.kernel.org, rednoax@gmail.com
Subject: Re: [PATCH] lib/idr.c: remove duplicated bound checking in
 sub_alloc
Message-Id: <20150320142803.c2f096eb607df8333a60dd25@linux-foundation.org>
In-Reply-To: <1426787827-1424-1-git-send-email-rednoax@qq.com>
References: <1426787827-1424-1-git-send-email-rednoax@qq.com>
X-Mailer: Sylpheed 3.4.1 (GTK+ 2.24.23; 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: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thu, 19 Mar 2015 17:57:07 +0000 Liu Sha <rednoax@qq.com> wrote:

> From: Liu Sha <rednoax@gmail.com>
> 
> The INT_MAX bound checking in sub_alloc checks two conditions to see
> whether the signed integer "id" is beyond INT_MAX:
> 
> 		if ((id >= MAX_IDR_BIT) || (id < 0))
> 			return -ENOSPC;
> 
> These two conditions are actually the same for "int" variable so one
> of them can be removed. If the above snippet is compiled with -Os option
> of gcc, only one checking will remain in disassembly code.
> 
> --- a/lib/idr.c
> +++ b/lib/idr.c
> @@ -262,7 +262,7 @@ static int sub_alloc(struct idr *idp, int *starting_id, struct idr_layer **pa,
>  			sh = IDR_BITS*l;
>  			id = ((id >> sh) ^ n ^ m) << sh;
>  		}
> -		if ((id >= MAX_IDR_BIT) || (id < 0))
> +		if (id >= MAX_IDR_BIT)
>  			return -ENOSPC;
>  		if (l == 0)
>  			break;

Well.  This only works because MAX_IDR_BIT happens to have unsigned
type, so the comparison is done with unsigned arithmetic.

The patch makes no difference to code size with my gcc and I'm inclined
to leave the code as-is for reasons of safety and clarity.