Unbalanced @push/@pull instructions in WriteListR()?

Technical Development Development Systems
1

I’m looking at routine WriteListR() from ListWriter.i6t. The routine makes use of a @push instruction to temporarily store the value of global variable listing_size as part of a else clause taking place within a larger for loop. Within the same else clause, there is an opportunity to jump out of the for loop entirely past the code that includes the @pull. It looks like when this happens, the matching @pull instruction will never occur.

[ WriteListR o depth from_start
	partition_classes partition_class_sizes
	...

	for (cl=1, memb=o, index=0, current_lt=EMPTY_TEXT_VALUE: groups_to_do>0: cl++) {
		...

		if ((LT_Compare(memb.list_together, lt_value) == 0) ||
		  (LT_Compare(memb.list_together, EMPTY_TEXT_VALUE) == 0))
			current_lt = EMPTY_TEXT_VALUE;
		else { ! current member LT meaningful and different from global LT
			if (LT_Compare(memb.list_together, current_lt) == 0) continue;

			! Otherwise this class begins a new group
			@push listing_size;									! <-- @push always happens if else clause entered
			q = memb; listing_size = 1; l = index; m = cl;
			while (m < no_classes && (LT_Compare(q.list_together, memb.list_together) == 0)) {
				m++; ! next class
				while (partition_classes->l ~= m) { ! find first member of next class
					l++; q = c_iterator(q, depth, lt_value, ADVANCE_ITF);
				}
				if (LT_Compare(q.list_together, memb.list_together) == 0)
					listing_size++;
			}

			if (listing_size > 1) {
				! The new group contains more than one partition class
				WriteMultiClassGroup(cl, memb, depth, partition_class_sizes);
				current_lt = memb.list_together;
				jump GroupComplete;								! <-- if branching here, creates unbalanced @push/@pull?
			}
			current_lt = EMPTY_TEXT_VALUE;
			@pull listing_size;									! <-- corresponding @pull happens here
		}

		WriteSingleClassGroup(cl, memb, depth, partition_class_sizes->cl);

		.GroupComplete;											! <-- jump target past block including @pull
		...
	}

	FreeStack(partition_class_sizes);
	FreeStack(partition_classes);
]; ! end of WriteListR
1 Like
2

It looks like this is true, but it doesn’t matter, because when returning from a routine anything that has been pushed to the stack but not pulled will be thrown away when the current stack frame is dismantled.

Z-Machine Standards 1.1 section 6.3.2 says:

The stack is left empty at the end of each routine: when a return occurs, any values pushed during the routine are thrown away.

and Glulx spec 0.7.5 section 1.3.3 says:

When a function returns, the process is reversed. First StackPtr is set back to FramePtr, throwing away the current call frame (and any pushed values).

Whenever a @pull is executed, it will be getting the value placed on the stack by the matching @push, and that’s the part that matters.

3

Leaving a value on the stack is untidy but safe. However, the point of the push/pull pair is to leave the value of listing_size unchanged at the time WriteListR() exits. (The listing_size variable is always handled this way.) By skipping over the pull, the code breaks that promise.

I don’t think it can cause a problem, but for consistency and clarity, it would be better to add a @pull listing_size; before the jump. (But after the WriteMultiClassGroup() call, as that function makes use of the computed listing_size value.)

4

Thank you for the clarification, zarf!

5

The other reason to fix it is that it took me twenty minutes of searching through an auto.inf file to figure out all the stuff I wrote above.

The next time someone notices this, it will take them twenty minutes of searching to figure out the same stuff. And so on forever until the function is fixed. Pure waste of everybody’s time.

Like I said in some other thread, the highest virtue is not having to worry about it.

2 Likes