Re: [BUG?] fetch into shallow sends a large number of objects

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Mar 08, 2016 at 08:25:24AM -0500, Jeff King wrote:

> > Good news. We have the mechanism in place, I think.
> > get_shallow_commits_by_rev_list() (from 'pu') will produce the right
> > shallow points for sending back to the client if you pass "--not
> > <current shallow points>" to it. It's meant to be used for
> > --shallow-exclude and --shallow-since, but if neither is given (nor
> > --depth) I guess we can run it with current shallow points. I wonder
> > if we can detect some common cases and avoid commit traversing this
> > way though.
> 
> I tried that, but I couldn't quite get it to work. I don't think we need
> any special rev-list, though; we can just find the boundary points of
> that traversal and mark them as new shallows.

By the way, I found a bug during my initial attempts with
get_shallow_commits_by_rev_list(). One of the loops uses "p" to traverse
a linked list, and then re-uses "p" again for another list traversal
inside the body of the loop. When the inner loop finishes, "p" is
left as NULL, and then the outer loop tries to access "p->next", which
segfaults.

I _think_ this is just a mistaken re-use of the variable, and can be
fixed with a new iteration variable for the inner loop, like:

diff --git a/shallow.c b/shallow.c
index 6ceb3f8..d600947 100644
--- a/shallow.c
+++ b/shallow.c
@@ -188,13 +188,14 @@ struct commit_list *get_shallow_commits_by_rev_list(int ac, const char **av,
 	 */
 	for (p = not_shallow_list; p; p = p->next) {
 		struct commit *c = p->item;
+		struct commit_list *parent;
 
 		if (parse_commit(c))
 			die("unable to parse commit %s",
 			    oid_to_hex(&c->object.oid));
 
-		for (p = c->parents; p; p = p->next)
-			if (!(p->item->object.flags & not_shallow_flag)) {
+		for (parent = c->parents; parent; parent = parent->next)
+			if (!(parent->item->object.flags & not_shallow_flag)) {
 				c->object.flags |= shallow_flag;
 				commit_list_insert(c, &result);
 				break;

As I said, I didn't end up using this function either way, but you
probably want the fix above for the rest of your series. :)

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]