From: Jeff King Date: Tue, 19 Mar 2013 13:25:29 GMT Subject: Re: [PATCH] index-pack: protect deepest_delta in multithread code Message-ID: <20130319132529.GA6646@sigill.intra.peff.net> In-Reply-To: <1363698075-12452-1-git-send-email-pclouds@gmail.com> On Tue, Mar 19, 2013 at 08:01:15PM +0700, Nguyen Thai Ngoc Duy wrote: > deepest_delta is a global variable but is updated without protection > in resolve_delta(), a multithreaded function. Add a new mutex for it, > but only protect and update when it's actually used (i.e. show_stat is > non-zero). This makes sense to me. > Another variable that will not be updated is delta_depth in "struct > object_entry" as it's only useful when show_stat is 1. Putting it in > "if (show_stat)" makes it clearer. Having just read through this code for the first time, I agree that having the "if (show_stat)" would have made it a lot more clear under what conditions and for what purpose the delta_depth flag was being used. > builtin/index-pack.c | 30 +++++++++++++++++++++++------- > 1 file changed, 23 insertions(+), 7 deletions(-) Patch looks good to me. Thanks. -Peff