Re: [PATCH] mailsplit and mailinfo: gracefully handle NUL characters

[Johannes Schindelin <Johannes.Schindelin@xxxxxx>]

Hi,

On Fri, 16 May 2008, Tommy Thorn wrote:

> Johannes Schindelin wrote:
> > +/*
> > + *  This is an ugly hack to avoid fgetc(), which is slow, as it is locking.
> > + *  The argument "in" must be the same for all calls to this function!
> > + */
> > +static int fast_fgetc(FILE *in)
> > +{
> >   
> 
> Looks great to me, but shouldn't you add an "inline" for this one? Also, 
> maybe a double the buffer size.

No.  This is an ugly hack, and not meant for application.

If that is substantially faster than the fgetc() version (and I want this 
be tested in a _real-world_ scenario, i.e. not the fgetc() alone, but a 
real mailsplit and a real mailinfo on a huge patch, with all three 
versions: fgets(), fgetc() and fast_fgetc())), then I would prefer having 
something like

	struct line_reader {
		FILE *in;
		char buffer[4096];
		int offset, int len;
		char line[1024];
		int linelen;
	};

and corresponding functions to read lines in that setting.  Maybe it would 
even be better to have line be a strbuf, but I am not so sure on that.

Let's see what the tests show.  Would you do them, please?  
"git format-patch --stdout bla..blub | /usr/bin/time git mailsplit -o." 
three times in succession should give you a good hint on the runtime.

Ciao,
Dscho


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