Re: [RFC PATCH 2/7] fetch-pack: allow NULL negotiator->known_common

> > Hmph, both the default and the skipping negotiator seem to put NULL
> > in known_common and add_tip when its next() method is called.  Also
> > they clear known_common to NULL after add_tip is called even once.
> >
> > So, how have we survived so far without this patch to "allow this
> > (i.e.  known_common method to be NULL)"?  Is there something that
> > makes sure a negotiator never gets called from this function after
> > its .next or .add_tip method is called?

> > Is the untold assumption that mark_complete_and_common_ref() will
> > never be called after either mark_tips() or find_common() have been
> > called?
> 
> Shot in the dark.  Perhaps clearing of .add_tip and .known_common in
> the .next method was done to catch a wrong calling sequence where
> mark_complete_and_common_ref() gets called after mark_tips() and/or
> find_common() have by forcing the code to segfault?

> If so, this
> patch removes the safety and we may want to add an equivalent safety
> logic.  Perhaps by adding a state field in the negotiator instance
> to record that mark_tips() and/or find_common() have been used and
> call a BUG() if mark_complete_and_common_ref() gets called after that,
> if enforcing such an invariant was the original reason why these
> fields were cleared.