From owner-freebsd-arch@FreeBSD.ORG Mon Oct 31 14:58:32 2011 Return-Path: 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 A4CD71065675 for ; Mon, 31 Oct 2011 14:58:32 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id 7DB858FC1D for ; Mon, 31 Oct 2011 14:58:32 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id 3396046B06; Mon, 31 Oct 2011 10:58:32 -0400 (EDT) Received: from jhbbsd.localnet (unknown [209.249.190.124]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id A27528A02E; Mon, 31 Oct 2011 10:58:31 -0400 (EDT) From: John Baldwin To: Jilles Tjoelker Date: Mon, 31 Oct 2011 10:24:07 -0400 User-Agent: KMail/1.13.5 (FreeBSD/8.2-CBSD-20110714-p8; KDE/4.5.5; amd64; ; ) References: <201110281426.00013.jhb@freebsd.org> <20111029214057.GB90408@stack.nl> In-Reply-To: <20111029214057.GB90408@stack.nl> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201110311024.07580.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.6 (bigwig.baldwin.cx); Mon, 31 Oct 2011 10:58:31 -0400 (EDT) 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 31 Oct 2011 14:58:32 -0000 On Saturday, October 29, 2011 5:40:58 pm Jilles Tjoelker wrote: > 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). Existing applications use the name and I find it ugly. (I also wish we had a plain fallocate() instead of just posix_fallocate().) However, if other folks prefer not having the wrapper I could update it to use the posix_* 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. Gah, fixed (I had thought of that case but forgot to update it when converting the advice to be malloc'd vs stored in struct file directly). > 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? -- John Baldwin