Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 29 Oct 2011 23:40:58 +0200
From:      Jilles Tjoelker <jilles@stack.nl>
To:        John Baldwin <jhb@freebsd.org>
Cc:        arch@freebsd.org
Subject:   Re: [PATCH] fadvise(2) system call
Message-ID:  <20111029214057.GB90408@stack.nl>
In-Reply-To: <201110281426.00013.jhb@freebsd.org>
References:  <201110281426.00013.jhb@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
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



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20111029214057.GB90408>