From owner-freebsd-arch@FreeBSD.ORG  Wed Nov  2 18:11:42 2011
Return-Path: <owner-freebsd-arch@FreeBSD.ORG>
Delivered-To: arch@freebsd.org
Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34])
	by hub.freebsd.org (Postfix) with ESMTP id D42BE1065673;
	Wed,  2 Nov 2011 18:11:42 +0000 (UTC) (envelope-from jilles@stack.nl)
Received: from mx1.stack.nl (relay04.stack.nl [IPv6:2001:610:1108:5010::107])
	by mx1.freebsd.org (Postfix) with ESMTP id 6F4B48FC15;
	Wed,  2 Nov 2011 18:11:42 +0000 (UTC)
Received: from snail.stack.nl (snail.stack.nl [IPv6:2001:610:1108:5010::131])
	by mx1.stack.nl (Postfix) with ESMTP id 5F1491DD743;
	Wed,  2 Nov 2011 19:11:41 +0100 (CET)
Received: by snail.stack.nl (Postfix, from userid 1677)
	id 473D728468; Wed,  2 Nov 2011 19:11:41 +0100 (CET)
Date: Wed, 2 Nov 2011 19:11:41 +0100
From: Jilles Tjoelker <jilles@stack.nl>
To: John Baldwin <jhb@freebsd.org>
Message-ID: <20111102181140.GA21621@stack.nl>
References: <201110281426.00013.jhb@freebsd.org>
	<20111029214057.GB90408@stack.nl>
	<201110311024.07580.jhb@freebsd.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <201110311024.07580.jhb@freebsd.org>
User-Agent: Mutt/1.5.21 (2010-09-15)
Cc: arch@freebsd.org
Subject: Re: [PATCH] fadvise(2) system call
X-BeenThere: freebsd-arch@freebsd.org
X-Mailman-Version: 2.1.5
Precedence: list
List-Id: Discussion related to FreeBSD architecture <freebsd-arch.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-arch>,
	<mailto:freebsd-arch-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-arch>
List-Post: <mailto:freebsd-arch@freebsd.org>
List-Help: <mailto:freebsd-arch-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-arch>,
	<mailto:freebsd-arch-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Wed, 02 Nov 2011 18:11:42 -0000

On Mon, Oct 31, 2011 at 10:24:07AM -0400, John Baldwin wrote:
> > The comparisons

> > +		    (fa->fa_start != 0 && fa->fa_start == end + 1) ||
> > +		    (uap->offset != 0 && fa->fa_end + 1 == uap->offset))) {

> > should instead be something like

> > +		    (end != OFF_MAX && fa->fa_start == end + 1) ||
> > +		    (fa->fa_end != OFF_MAX && fa->fa_end + 1 == uap->offset))) {

> > to avoid integer overflow.

> Hmm, but the expressions will still work in that case, yes?  I already
> check for uap->offset and uap->len being negative earlier (so fa_start
> and fa_end are always positive), and off_t is signed, so if end is
> OFF_MAX, then end + 1 will certainly not == fa_start?

Signed integer overflow is undefined behaviour; therefore, if you write
end + 1 without checking that end != OFF_MAX, the compiler may assume
that end != OFF_MAX. Whether the compiler will take advantage of this in
ways that cause breakage is another question. For example, if there were
a subsequent check for end != OFF_MAX, the compiler would be allowed to
remove that check. I think it is best not to risk it.

-- 
Jilles Tjoelker