Git Mailing List Archive mirror
 help / color / mirror / Atom feed
From: Han Xin <hanxin.hx@bytedance.com>
To: git@vger.kernel.org
Cc: Han Xin <hanxin.hx@bytedance.com>,
	xingxin.xx@bytedance.com, jonathantanmy@google.com,
	derrickstolee@github.com, Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v2 0/2] negotiator/default: avoid stack overflow
Date: Wed, 26 Apr 2023 21:15:02 +0800	[thread overview]
Message-ID: <cover.1682513384.git.hanxin.hx@bytedance.com> (raw)
In-Reply-To: <cover.1682473718.git.hanxin.hx@bytedance.com>

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 <xingxin.xx@bytedance.com>
         Signed-off-by: Han Xin <hanxin.hx@bytedance.com>
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


  parent reply	other threads:[~2023-04-26 13:15 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-24  2:23 [PATCH v1] negotiator/default.c: avoid stack overflow Han Xin
2023-04-24 14:44 ` Derrick Stolee
2023-04-25  3:02   ` [External] " Han Xin
2023-04-25 13:34     ` Derrick Stolee
2023-04-26  4:05 ` [PATCH v2 0/2] negotiator/default: " Han Xin
2023-04-26  4:05   ` [PATCH v2 1/2] " Han Xin
2023-04-26 11:13     ` Derrick Stolee
2023-04-26 11:40       ` [External] " Han Xin
2023-04-26  4:05   ` [PATCH v2 2/2] negotiator/skipping: fix some problems in mark_common() Han Xin
2023-04-26 11:08     ` Derrick Stolee
2023-04-26 11:55       ` [External] " Han Xin
2023-04-26 13:15   ` Han Xin [this message]
2023-04-26 13:15     ` [PATCH v3 1/2] negotiator/default: avoid stack overflow Han Xin
2023-04-26 17:14       ` Junio C Hamano
2023-04-26 17:30         ` Derrick Stolee
2023-04-26 17:38           ` Junio C Hamano
2023-04-26 13:15     ` [PATCH v3 2/2] negotiator/skipping: fix some problems in mark_common() Han Xin
2023-05-01 22:11     ` [PATCH v2 0/2] negotiator/default: avoid stack overflow Junio C Hamano
2023-05-02  1:49       ` Derrick Stolee
2023-05-02 15:51         ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1682513384.git.hanxin.hx@bytedance.com \
    --to=hanxin.hx@bytedance.com \
    --cc=derrickstolee@github.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jonathantanmy@google.com \
    --cc=xingxin.xx@bytedance.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).