Probable bug in file run-command.c function clear_child_for_cleanup

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

This is probably the wrong way to do this, and I'm sorry if I end up wasting someone's time. That said, here goes....

While idly browsing through the git source (as you do on a sunny Sunday afternoon), I spotted the following code (that appears to be wrong) in the file https://github.com/git/git/blob/master/run-command.c It's the same in branches maint, next and pu. The branch todo gives me a 404.

(line 53 is here)
static void clear_child_for_cleanup(pid_t pid)
{
	struct child_to_clean **last, *p;

	last = &children_to_clean;
	for (p = children_to_clean; p; p = p->next) {
		if (p->pid == pid) {
			*last = p->next;
			free(p);
			return;
		}
	}
}
(line 67 is here)

It appears that last is intended to point to the next field that's being updated, but fails to "follow" the p pointer along the chain. The result is that children_to_clean will end up pointing to the entry after the deleted one, and all the entries before it will be lost. It'll only be fine in the case that the pid is that of the first entry in the chain.

You want something like:

for (... {
	if (... {
		...
	}
	last = &p->next;
}

or (probably clearer, but I haven't read your coding style guide, if there is one, and some people don't like this approach)

for (p = children_to_clean; p; last = &p->next, p = p->next) {
	...

Cheers,
David

--
David Gould, Personal Trainer
	Register of Kettlebell Professionals
	INWA Nordic Walking Instructor
Optimise Fitness Ltd -- fit for life
01264 720709
www.optimisefitness.com
--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]