Re: [PATCH v3 4/4] commit: rewrite read_graft_line

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 18, 2017, 16:44 UTC
Message-ID: <xmqqd17sddvz.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAJfL8+SHSAhgrMY6ONVHLMWEHcT0mhm4oKMmq6D=89SErDKiMA@mail.gmail.com>

Patryk Obara <patryk.obara@gmail.com> writes:

Show 9 quoted lines

> Junio C Hamano <gitster@pobox.com> wrote:
>>
>> If I were doing the two-pass thing, I'd probably write a for loop
>> that runs exactly twice, where the first iteration parses into a
>> single throw-away oid struct only to count, and the second iteration
>> parses the same input into the allocated array of oid struct.  That
>> way, you do not have to worry about two phrases going out of sync.
>
> Two passes would still differ in error handling due to xmalloc between them…

I am not sure if I follow. What I meant was something along these lines:

	struct commit_graft *graft = NULL;
        char *line = ... what you read from the file ...;
        int phase; /* phase #0 counts, phase #1 fills */

	for (phase = 0; phase < 2; phase++) {
		int count;
		char *scan;
		strucut object_id dummy_oid, *oid;

		for (scan = line, count = 0;
                     *scan;
		     count++) {
                	oid = graft ? &graft->parent[count] : &dummy_oid;
			if (parse_oid_hex(scan, oid, &scan))
				return error(...);
			switch (*scan) {
			case ' ':
				scan++;
				continue; /* there are more */
			case '\0':
				break; /* we are done */
			default:
				return error(...);
			}
		}

		if (!graft)
			graft = xmalloc(... with 'count' parentes ...);
	}

        /* now we have graft with parent[count] all filled */
	return graft;

The inner for() loop will do the same parsing for both passes, leaving little chance for programming errors to make the two passes decide there are different number of fake parents. I suspect I may have botched some details in that loop, but both passes will even share the same buggy counting when the code is structured that way ;-)

That is what I meant by "not have to worry about two phases going out of sync".