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

2 messages, 2 authors, 2017-08-18 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2017-08-18 16:44:39

Patryk Obara [off-list ref] writes:
Junio C Hamano [off-list ref] wrote:
quoted
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".

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

From: Patryk Obara <hidden>
Date: 2017-08-18 17:06:32

Ah! I presumed two separate loops, one throwing away oids and second
one actually filling a table - this makes more sense. I was just about
to send v4, but will rewrite the last patch and we'll see how it looks
like.
-- 
| ← Ceci n'est pas une pipe
Patryk Obara
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help