On Thu, Feb 25, 2016 at 10:02 AM, Junio C Hamano <gits...@pobox.com> wrote: > Duy Nguyen <pclo...@gmail.com> writes: > >> On Thu, Feb 25, 2016 at 5:09 AM, Christian Couder >> <christian.cou...@gmail.com> wrote: >>> Another possibility would be to libify the "git apply" functionality >>> and then to use the libified "git apply" in run_apply() instead of >>> launching a separate "git apply" process. One benefit from this is >>> that we could probably get rid of the read_cache_from() call at the >>> end of run_apply() and this would likely further speed up things. Also >>> avoiding to launch separate processes might be a win especially on >>> Windows. >> >> The amount of global variables in apply.c is just scary. Libification >> will need some cleanup first (i'm not against it though). > > The global variables do not scare me. You can just throw them into > a single "apply_state" structure and pass it around the callchain > just fine--the helper functions in the file all take only a small > number of parameters, and a new parameter that is a pointer to the > state structure that consistently comes at the beginning would not > make things harder to read or maintain.
There are a bunch of shadow variables in this file. If you are not careful, it's easy to mistake local var "x" with global "x" and replace it with global_state->x. > What does scare me (and what you should be scared) more is the way > the current code handles erroneous input. It was one of the > programs written in early days with "just do one thing well and > exit, the larger program structure will be scripted around us" > mentality and liberally die() without cleaning things up. I do > agree with others that libification of it is a very good thing, but > the second step (the first step is the easier "global to state > structure" one, which should be more or less mechanical) towards it > is to design how the errors are handled and resources are released. Yeah.. thorough study of apply code may be needed before anybody does any surgery in this code -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
