Re: [PATCH] Replacing the system call pread() with lseek()/xread()/lseek() sequence.

From: Junio C Hamano <junkio@cox.net>
Date: Jan 9, 2007, 20:21 UTC
Message-ID: <7v1wm4kl24.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <41319.3372231492$1168368741@news.gmane.org>

"Stefan-W. Hahn" <stefan.hahn@s-hahn.de> writes:

Show 5 quoted lines

> 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>

Thanks.

> +# Define NO_PREAD if you have a problem with pread() system call (i.e.
> +# cygwin.dll before v1.5.22).
> +#

I would have preferred e.g. not i.e..

Show 9 quoted lines

> diff --git a/compat/pread.c b/compat/pread.c
> new file mode 100644
> index 0000000..cd1da87
> --- /dev/null
> +++ b/compat/pread.c
> @@ -0,0 +1,15 @@
> +#include "../git-compat-util.h"
> +
> +ssize_t git_pread(int fd,void *buf,size_t count,off_t offset)

Style: s/,/, /; applies to git-compat-util.h as well.

Show 7 quoted lines

> +{
> +        off_t current_offset = lseek(fd, 0, SEEK_CUR);
> +
> +        if (lseek(fd, offset, SEEK_SET) < 0)
> +                return EINVAL;
> +
> +        ssize_t rc=xread(fd, buf, count);

Style: please don't have declaration after statement.  Yes, it
is in the more recent ANSI, but the rest of git has avoided it
so far and having the decls upfront makes it easier to read.

I do not think you would want to return EINVAL.

Andy's read_in_full() is now on 'master' so please use it instead to avoid short read.