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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



"Shawn O. Pearce" <spearce@xxxxxxxxxxx> writes:

> With the exception of this style difference, the patch looked
> pretty good.  Nice work Stefan.  Andy's right, we do tend to prefer
> "rc = read_in_full" over "rc=read_in_full".  Quite a bit actually,
> though Junio is the final decider on all such matters as he gets
> to choose to accept or reject the patch.  ;-)

I am a nice guy and do not reject a patch for missing two SP
characters which means I have to --amend it which takes time
away from me.  Maybe I should stop being nice ;-).

>> > +
>> > +        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.
>
> Andy's right actually.  If we are using pread() we aren't relying
> on the current file pointer.  Which means its unnecessary to get
> the current pointer before seeking to the requested offset, and its
> unnecessary to restore it before the git_pread() function returns.

The caller of pread() does not care the current position, but
that is not to mean it does not care the position after pread()
returns.  The current callers do not care, though.

-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]