Skip to contentSkip to conversation

get_indexed_object can return NULL if nothing is in that slot; check for it

PATCH In review Brian Harring 2 participants 4 messages Jun 15, 2016

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 get_indexed_object can return NULL if nothing is in that slot; check for it

This fixes a segfault introduced by 051e400; via it, no longer able to trigger the http/smartserv race.

---
 upload-pack.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
upload-pack.c +2 -0
index 6420918..8739bfa 100644
@@ -533,6 +533,8 @@ static void check_non_tip(void)
 	namebuf[41] = '\n';
 	for (i = get_max_object_index(); 0 < i; ) {
 		o = get_indexed_object(--i);
+		if (!o)
+			continue;
 		if (!(o->flags & OUR_REF))
 			continue;
 		memcpy(namebuf + 1, sha1_to_hex(o->sha1), 40);
Signature · 1 line
1.7.6
#1 lore raw

Thanks for a fix.

It is both interesting and disturbing to see that these small mistakes are discovered a week after the topics hit 'master', even though it has been cooking in 'next' for a week before that happened (in the case of this topic, it also hit 'maint' yesterday).

#2 lore raw
Quoted 6 lines · Junio C Hamano

Thanks for a fix.

It is both interesting and disturbing to see that these small mistakes are discovered a week after the topics hit 'master', even though it has been cooking in 'next' for a week before that happened (in the case of this topic, it also hit 'maint' yesterday).

I'll admit I'm slightly surprised it slipped past for initial development- that said, you have to explicitly trigger the race to trigger the segfault. And that's not easy w/out building out a custom setup- even w/ that setup, you need to go digging in server logs to realize the previous serverside failure just converted to a segfault. Clientside, it hung just the same.

Bit nonobvious. Plus, shit happens. ;)

Either way, I was poking at the source trying to figure out how to get some unittests for an end to end testing of the http/smartserv; that said I was having a helluva time finding a way to do it without bundling a stub of a webserver. Suggestions would be welcome on that one.

~brian

#3 lore raw
Quoted 15 lines · Brian Harring
Quoted 6 lines · Junio C Hamano

Thanks for a fix.

It is both interesting and disturbing to see that these small mistakes are discovered a week after the topics hit 'master', even though it has been cooking in 'next' for a week before that happened (in the case of this topic, it also hit 'maint' yesterday).

I'll admit I'm slightly surprised it slipped past for initial development- that said, you have to explicitly trigger the race to trigger the segfault. And that's not easy w/out building out a custom setup- even w/ that setup, you need to go digging in server logs to realize the previous serverside failure just converted to a segfault. Clientside, it hung just the same.

Quoted 7 lines

Bit nonobvious. Plus, shit happens. ;)

Either way, I was poking at the source trying to figure out how to get some unittests for an end to end testing of the http/smartserv; that said I was having a helluva time finding a way to do it without bundling a stub of a webserver. Suggestions would be welcome on that one.

I probably should have made it clear that I was not complaining, and if it sounded as if I was complaining at _you_, my deepest apologies.

In any case, thanks, and congratulations for your first commit in git.git ;-)