From: Andy Whitcroft <apw@shadowen•org>
To: "Stefan-W. Hahn" <stefan.hahn@s-hahn•de>
Cc: git@vger•kernel.org
Subject: Re: [PATCH] Replacing the system call pread() with lseek()/xread()/lseek() sequence.
Date: Tue, 09 Jan 2007 21:41:41 +0000 [thread overview]
Message-ID: <45A40C15.1070200@shadowen.org> (raw)
In-Reply-To: <11683766521544-git-send-email->
Stefan-W. Hahn wrote:
> From: Stefan-W. Hahn <stefan.hahn@s-hahn•de>
>
> Using cygwin with cygwin.dll before 1.5.22 the system call pread() is buggy.
> This patch introduces NO_PREAD. If NO_PREAD is set git uses a sequence of
> lseek()/xread()/lseek() to emulate pread.
>
> Signed-off-by: Stefan-W. Hahn <stefan.hahn@s-hahn•de>
> ---
> Makefile | 7 +++++++
> compat/pread.c | 18 ++++++++++++++++++
> git-compat-util.h | 5 +++++
> 3 files changed, 30 insertions(+), 0 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 6c12bc6..43113e9 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -69,6 +69,9 @@ all:
> #
> # Define NO_MMAP if you want to avoid mmap.
> #
> +# Define NO_PREAD if you have a problem with pread() system call (e.g.
> +# cygwin.dll before v1.5.22).
> +#
> # Define NO_FAST_WORKING_DIRECTORY if accessing objects in pack files is
> # generally faster on your platform than accessing the working directory.
> #
> @@ -523,6 +526,10 @@ ifdef NO_MMAP
> COMPAT_CFLAGS += -DNO_MMAP
> COMPAT_OBJS += compat/mmap.o
> endif
> +ifdef NO_PREAD
> + COMPAT_CFLAGS += -DNO_PREAD
> + COMPAT_OBJS += compat/pread.o
> +endif
> ifdef NO_FAST_WORKING_DIRECTORY
> BASIC_CFLAGS += -DNO_FAST_WORKING_DIRECTORY
> endif
> diff --git a/compat/pread.c b/compat/pread.c
> new file mode 100644
> index 0000000..9183c05
> --- /dev/null
> +++ b/compat/pread.c
> @@ -0,0 +1,18 @@
> +#include "../git-compat-util.h"
> +
> +ssize_t git_pread(int fd, void *buf, size_t count, off_t offset)
> +{
> + off_t current_offset;
> + ssize_t rc;
> +
> + current_offset = lseek(fd, 0, SEEK_CUR);
> +
> + if (lseek(fd, offset, SEEK_SET) < 0)
> + return -1;
> +
> + rc=read_in_full(fd, buf, count);
Seems to be style inconsistancy between current_offset = and rc= I
believe the former is preferred.
> +
> + if (current_offset != lseek(fd, current_offset, SEEK_SET))
> + return -1;
How likely are we ever to be in the right place here? Seems vanishingly
small putting us firmly in the four syscalls per call space. I wonder
if git ever actually cares about the seek location. ie if we could stop
reading and resetting it. Probabally not worth working it out I guess
as any _sane_ system has one.
> + return rc;
> +}
> diff --git a/git-compat-util.h b/git-compat-util.h
> index e023bf1..f8d46d5 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -107,6 +107,11 @@ extern int git_munmap(void *start, size_t length);
> #define DEFAULT_PACKED_GIT_LIMIT \
> ((1024L * 1024L) * (sizeof(void*) >= 8 ? 8192 : 256))
>
> +#ifdef NO_PREAD
> +#define pread git_pread
> +extern ssize_t git_pread(int fd, void *buf, size_t count, off_t offset);
> +#endif
> +
> #ifdef NO_SETENV
> #define setenv gitsetenv
> extern int gitsetenv(const char *, const char *, int);
-apw
next prev parent reply other threads:[~2007-01-09 21:41 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <11683766523955-git-send-email->
2007-01-09 21:04 ` [PATCH] Replacing the system call pread() with lseek()/xread()/lseek() sequence Stefan-W. Hahn
[not found] ` <11683766521544-git-send-email->
2007-01-09 21:41 ` Andy Whitcroft [this message]
2007-01-09 23:25 ` Shawn O. Pearce
2007-01-10 0:21 ` Junio C Hamano
2007-01-10 0:30 ` Johannes Schindelin
2007-01-10 1:12 ` Nicolas Pitre
2007-01-09 23:42 ` Johannes Schindelin
2007-01-10 0:59 ` Nicolas Pitre
2007-01-09 21:04 Stefan-W. Hahn
[not found] <11683687161816-git-send-email-@videotron.ca>
[not found] ` <11683687162492-git-send-email-@videotron.ca>
[not found] ` <11683687161239-git-send-email-@videotron.ca>
2007-01-09 19:48 ` Nicolas Pitre
[not found] <11683687161816-git-send-email->
[not found] ` <11683687162492-git-send-email->
2007-01-07 16:36 ` [PATCH] Replacing the system call pread() with real mmap() Stefan-W. Hahn
2007-01-09 18:51 ` [PATCH] Replacing the system call pread() with lseek()/xread()/lseek() sequence Stefan-W. Hahn
2007-01-09 20:21 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=45A40C15.1070200@shadowen.org \
--to=apw@shadowen$(echo .)org \
--cc=git@vger$(echo .)kernel.org \
--cc=stefan.hahn@s-hahn$(echo .)de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox