From owner-freebsd-arch@FreeBSD.ORG  Sat Oct 29 21:40:59 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 EC64B1065670;
	Sat, 29 Oct 2011 21:40:59 +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 8BB3B8FC08;
	Sat, 29 Oct 2011 21:40:59 +0000 (UTC)
Received: from snail.stack.nl (snail.stack.nl [IPv6:2001:610:1108:5010::131])
	by mx1.stack.nl (Postfix) with ESMTP id 50AE31DD59C;
	Sat, 29 Oct 2011 23:40:58 +0200 (CEST)
Received: by snail.stack.nl (Postfix, from userid 1677)
	id 2A2E228468; Sat, 29 Oct 2011 23:40:58 +0200 (CEST)
Date: Sat, 29 Oct 2011 23:40:58 +0200
From: Jilles Tjoelker <jilles@stack.nl>
To: John Baldwin <jhb@freebsd.org>
Message-ID: <20111029214057.GB90408@stack.nl>
References: <201110281426.00013.jhb@freebsd.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <201110281426.00013.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: Sat, 29 Oct 2011 21:41:00 -0000

On Fri, Oct 28, 2011 at 02:25:59PM -0400, John Baldwin wrote:
> I have been working for the last week or so on a patch to add an
> fadvise(2) system call.  It is somewhat similar to madvise(2) except
> that it operates on a file descriptor instead of a memory region.  It
> also only really makes sense for regular files and does not apply to
> other file descriptor types.

Cool.

Various other posix_* functions such as posix_spawn() and posix_openpt()
are implemented directly, not as a wrapper around s/posix_//. I think
posix_madvise() is only implemented as a wrapper because madvise()
already existed. Therefore, I don't see a reason why a function named
fadvise would be useful on its own (except if there are existing
applications that use that name).

If the advice is FADV_SEQUENTIAL, FADV_RANDOM or FADV_NOREUSE and the
file descriptor is invalid (fget() fails), the struct fadvise_info is
leaked.

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.

-- 
Jilles Tjoelker