Re: [PATCH 3/6] strbuf_getwholeline: use getc_unlocked

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

 



On Sun, Apr 05, 2015 at 01:27:32AM -0400, Jeff King wrote:

> On Sun, Apr 05, 2015 at 12:56:14AM -0400, Jeff King wrote:
> 
> > The big downside is that our input strings are no longer NUL-clean
> > (reading "foo\0bar\n" would yield just "foo". I doubt that matters in
> > the real world, but it does fail a few of the tests (e.g., t7008 tries
> > to read a list of patterns which includes NUL, and we silently truncate
> > the pattern rather than read in the NUL and barf).
> 
> So there is this trick:
> 
> diff --git a/strbuf.c b/strbuf.c
> index f319d8d..5ceebb7 100644
> --- a/strbuf.c
> +++ b/strbuf.c
> @@ -445,12 +445,13 @@ int strbuf_getwholeline(struct strbuf *sb, FILE *fp, int term)
>  	strbuf_reset(sb);
>  
>  	if (term == '\n') {
> +		long pos = ftell(fp);
>  		strbuf_grow(sb, 256);
>  		if (!fgets(sb->buf, sb->alloc - 1, fp)) {
>  			strbuf_release(sb);
>  			return EOF;
>  		}
> -		sb->len = strlen(sb->buf);
> +		sb->len = ftell(fp) - pos;
>  		if (sb->buf[sb->len - 1] == '\n')
>  			return 0;
>  	}
> 
> but much to my surprise it actually runs slower than the strlen version!
> It also has a 32-bit overflow issue. There's fgetpos() as an
> alternative, but fpos_t is an opaque type, and we might not be able to
> do arithmetic on it (for that matter, I am not sure if arithmetic is
> strictly guaranteed on ftell() results). POSIX gives us ftello(), which
> returns an off_t. That would probably be fine.

Actually, scratch that idea. ftell() always returns 0 on a non-seekable
file, so we can't use it in the general case. And that probably explains
the performance difference, too, if it is not keeping its own counter
and relies on lseek(fileno(fp)) or similar.

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