threads / patch / 28203

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

Subject: [PATCH] get_indexed_object can return NULL if nothing is in that slot; check for it

## tl;dr

4 messages between Aug 24, 2011 and Aug 24, 2011. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Brian Harring· Aug 24, 2011, 05:47 UTC · lore

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

Signed-off-by: Brian Harring <ferringb@gmail.com>
---
 upload-pack.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

Show changes to upload-pack.c +2 −0

diff --git a/upload-pack.c b/upload-pack.c
index 6420918..8739bfa 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -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);

-- 
1.7.6

Junio C Hamano· Aug 24, 2011, 17:54 UTC · re: Brian Harring · lore

Re: [PATCH] get_indexed_object can return NULL if nothing is in that slot; check for it

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).

Brian Harring· Aug 24, 2011, 20:40 UTC · re: Junio C Hamano · lore

Re: [PATCH] get_indexed_object can return NULL if nothing is in that slot; check for it

On Wed, Aug 24, 2011 at 10:54:51AM -0700, Junio C Hamano wrote:

Show 6 quoted lines

> 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

Junio C Hamano· Aug 24, 2011, 21:10 UTC · re: Brian Harring · lore

Re: [PATCH] get_indexed_object can return NULL if nothing is in that slot; check for it

Brian Harring <ferringb@gmail.com> writes:

Show 22 quoted lines

> On Wed, Aug 24, 2011 at 10:54:51AM -0700, Junio C Hamano wrote:
>> 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.

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 ;-)