@@ -24,3 +24,14 @@ void fetch_negotiator_init(struct repository *r,
return;
}
}
+
+void known_common_BUG(struct fetch_negotiator *negotiator,
+ struct commit *commit)
+{
+ BUG("known_common() called after add_tip() and/or next() was called");
+}
+
+void add_tip_BUG(struct fetch_negotiator *negotiator, struct commit *commit)
+{
+ BUG("add_tip() called after next() called");
+}
@@ -28,6 +28,9 @@ struct fetch_negotiator {
* Once this function is invoked, known_common() cannot be invoked any
* more.
*
+ * Set "known_common" to "known_common_BUG" in this callback
+ * to assert the invocation flow.
+ *
* Indicate that this commit and all its ancestors are to be checked
* for commonality with the server.
*/
@@ -37,6 +40,10 @@ struct fetch_negotiator {
* Once this function is invoked, known_common() and add_tip() cannot
* be invoked any more.
*
+ * Set "add_tip" to "add_tip_BUG" in this callback to assert
+ * the invocation flow, and "known_common" to
+ * "known_common_BUG" as noted for in add_tip() above.
+ *
* Return the next commit that the client should send as a "have" line.
*/
const struct object_id *(*next)(struct fetch_negotiator *);
@@ -56,4 +63,7 @@ struct fetch_negotiator {
void fetch_negotiator_init(struct repository *r,
struct fetch_negotiator *negotiator);
+void known_common_BUG(struct fetch_negotiator *, struct commit *);
+void add_tip_BUG(struct fetch_negotiator *, struct commit *);
+
#endif
@@ -135,14 +135,14 @@ static void known_common(struct fetch_negotiator *n, struct commit *c)
static void add_tip(struct fetch_negotiator *n, struct commit *c)
{
- n->known_common = NULL;
+ n->known_common = known_common_BUG;
rev_list_push(n->data, c, SEEN);
}
static const struct object_id *next(struct fetch_negotiator *n)
{
- n->known_common = NULL;
- n->add_tip = NULL;
+ n->known_common = known_common_BUG;
+ n->add_tip = add_tip_BUG;
return get_rev(n->data);
}
@@ -11,10 +11,13 @@ static void known_common(struct fetch_negotiator *n, struct commit *c)
static void add_tip(struct fetch_negotiator *n, struct commit *c)
{
/* do nothing */
+ n->known_common = known_common_BUG;
}
static const struct object_id *next(struct fetch_negotiator *n)
{
+ n->known_common = known_common_BUG;
+ n->add_tip = add_tip_BUG;
return NULL;
}
@@ -204,7 +204,7 @@ static void known_common(struct fetch_negotiator *n, struct commit *c)
static void add_tip(struct fetch_negotiator *n, struct commit *c)
{
- n->known_common = NULL;
+ n->known_common = known_common_BUG;
if (c->object.flags & SEEN)
return;
rev_list_push(n->data, c, 0);
@@ -212,8 +212,8 @@ static void add_tip(struct fetch_negotiator *n, struct commit *c)
static const struct object_id *next(struct fetch_negotiator *n)
{
- n->known_common = NULL;
- n->add_tip = NULL;
+ n->known_common = known_common_BUG;
+ n->add_tip = add_tip_BUG;
return get_rev(n->data);
}
As noted in ec06283844a (fetch-pack: introduce negotiator API, 2018-06-14) it's important that the fetch negotiator's callbacks be called in the documented order, and that some of them never be called again after other "later" callbacks are called. But let's assert that with a BUG(), instead of setting the relevant callbacks to NULL. We'll now give a meaningful error on API misuse, instead of segfaulting. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- fetch-negotiator.c | 11 +++++++++++ fetch-negotiator.h | 10 ++++++++++ negotiator/default.c | 6 +++--- negotiator/noop.c | 3 +++ negotiator/skipping.c | 6 +++--- 5 files changed, 30 insertions(+), 6 deletions(-)