Subject: [PATCH v2 0/2] negotiator/default: avoid stack overflow

From: Han Xin <[email-masked]>
Date: Wed, 26 Apr 2023 21:15:02 +0800

This series avoid stack overflow in negotiator/default.c and memory leak
in negotiator/skipping.c.

Changes since v2:
* Rewrite the commit link in the typical format.
* Fix the incorrect check for the COMMON bit introduced in v2.

Han Xin (2):
  negotiator/default: avoid stack overflow
  negotiator/skipping: fix some problems in mark_common()

 negotiator/default.c  | 39 +++++++++++++++++++++++++++++----------
 negotiator/skipping.c | 22 +++++++++++++++-------
 2 files changed, 44 insertions(+), 17 deletions(-)

Range-diff against v2:
1:  935be72eb9 ! 1:  0e69d70805 negotiator/default: avoid stack overflow
    @@ Commit message
         recursive function calls. Avoid this by instead recursing using a
         heap-allocated data structure.
     
    -    This is the same case as [1].
    -
    -    1. 4654134976f (negotiator/skipping: avoid stack overflow, 2022-10-25)
    +    This is the same case as 4654134976f (negotiator/skipping: avoid
    +    stack overflow, 2022-10-25)
     
         Reported-by: Xin Xing <[email-masked]>
         Signed-off-by: Han Xin <[email-masked]>
2:  abbb1bc0b3 ! 2:  8b5c92a4d5 negotiator/skipping: fix some problems in mark_common()
    @@ Metadata
      ## Commit message ##
         negotiator/skipping: fix some problems in mark_common()
     
    -    Fixed the following problems:
    +    The mark_common() method in negotiator/skipping.c was converted
    +    from recursive to iterative in 4654134976f (negotiator/skipping:
    +    avoid stack overflow, 2022-10-25), but there is some more work
    +    to do:
     
         1. prio_queue() should be used with clear_prio_queue(), otherwise there
            will be a memory leak.
    @@ negotiator/skipping.c: static int clear_marks(const char *refname, const struct
       */
      static void mark_common(struct data *data, struct commit *seen_commit)
      {
    -@@ negotiator/skipping.c: static void mark_common(struct data *data, struct commit *seen_commit)
    + 	struct prio_queue queue = { NULL };
    + 	struct commit *c;
    + 
    ++	if (seen_commit->object.flags & COMMON)
    ++		return;
    ++
    + 	prio_queue_put(&queue, seen_commit);
    ++	seen_commit->object.flags |= COMMON;
      	while ((c = prio_queue_get(&queue))) {
      		struct commit_list *p;
    - 		if (c->object.flags & COMMON)
    +-		if (c->object.flags & COMMON)
     -			return;
    -+			continue;
    - 		c->object.flags |= COMMON;
    +-		c->object.flags |= COMMON;
    ++
      		if (!(c->object.flags & POPPED))
      			data->non_common_revs--;
      
    @@ negotiator/skipping.c: static void mark_common(struct data *data, struct commit
     +			continue;
      		for (p = c->parents; p; p = p->next) {
     -			if (p->item->object.flags & SEEN)
    -+			if (p->item->object.flags & SEEN || p->item->object.flags & COMMON)
    - 				prio_queue_put(&queue, p->item);
    +-				prio_queue_put(&queue, p->item);
    ++			if (!(p->item->object.flags & SEEN) ||
    ++			    (p->item->object.flags & COMMON))
    ++				continue;
    ++
    ++			p->item->object.flags |= COMMON;
    ++			prio_queue_put(&queue, p->item);
      		}
      	}
     +
-- 
2.40.0