Re: [PATCH] Remove the line length limit for graft files

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Dec 27, 2013, 21:14 UTC
Message-ID: <alpine.DEB.1.00.1312272208070.1191@s15462909.onlinehome-server.info>
In-Reply-To: <20131227210447.GE20443@google.com>

Hi,

On Fri, 27 Dec 2013, Jonathan Nieder wrote:

Show 10 quoted lines

> Johannes Schindelin wrote:
> 
> [...]
> > ---
> >  builtin/blame.c |  8 ++++----
> >  commit.c        | 10 +++++-----
> >  2 files changed, 9 insertions(+), 9 deletions(-)
> 
> Is this easy to reproduce so some interested but lazy person could
> write a test?

Yep. Make 25 orphan commits, add a graft line to make the first a merge of the rest.

Show 17 quoted lines

> > --- a/builtin/blame.c
> > +++ b/builtin/blame.c
> > @@ -1804,17 +1804,17 @@ static int prepare_lines(struct scoreboard *sb)
> >  static int read_ancestry(const char *graft_file)
> >  {
> >  	FILE *fp = fopen(graft_file, "r");
> > -	char buf[1024];
> > +	struct strbuf buf = STRBUF_INIT;
> >  	if (!fp)
> >  		return -1;
> > -	while (fgets(buf, sizeof(buf), fp)) {
> > +	while (!strbuf_getwholeline(&buf, fp, '\n')) {
> 
> If there is no newline at EOF, this will skip the last line, while the
> old behavior was to pay attention to it.  I haven't thought through
> whether that's a good or bad change.  Maybe it should just be
> documented?

The way I read

	int strbuf_getwholeline(struct strbuf *sb, FILE *fp, int term)
	{
		int ch;

		if (feof(fp))
			return EOF;

		strbuf_reset(sb);
		while ((ch = fgetc(fp)) != EOF) {
			strbuf_grow(sb, 1);
			sb->buf[sb->len++] = ch;
			if (ch == term)
				break;
		}
		if (ch == EOF && sb->len == 0)
			return EOF;

		sb->buf[sb->len] = '\0';
		return 0;
	}

it returns EOF only if ch == EOF *and* sb->len == 0, i.e. if no characters have been read before hitting EOF.

In other words, strbuf_getwholeline() -- despite requiring an explicit terminating character argument -- does not require the last line to end with that terminating character.

A quick test (in my case, because I am lazy, modifying test-mergesort.c to output the lines that were read by strbuf_getwholeline()) also confirms my suspicion.

Or maybe I missed something?

Ciao, Dscho

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.