Andrey Okoshkin <a.okoshkin@xxxxxxxxxxx> writes:
> Get 'GIT_MERGE_VERBOSITY' environment variable only once in
> init_merge_options() and store the pointer to its value for the further check.
OK, that is "what this thing does" description.
> No intervening calls to getenv(), putenv(), setenv() or unsetenv() are done
> between the initial getenv() call and the consequential result pass to strtol()
> as these environment related functions could modify the string pointer returned
> by the initial getenv() call.
That holds true for the code before or after this patch equally. In
other words, that sounds like a justification for rejecting this
patch (i.e. explanation of why this change is not needed).
If we are worried about another thread calling these functions after
the time we call getenv() and before the time we pass the result to
strtol(), then I do not think this patch gives a better protection
against such race, so I do not think that is why you are doing this.
So... why do we want to do this change? I am puzzled.
>
> Signed-off-by: Andrey Okoshkin <a.okoshkin@xxxxxxxxxxx>
> Reviewed-by: Stefan Beller <sbeller@xxxxxxxxxx>
> ---
> Added 'reviewed-by' field.
> merge-recursive.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/merge-recursive.c b/merge-recursive.c
> index 1494ffdb8..60084e3a0 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -2163,6 +2163,7 @@ static void merge_recursive_config(struct merge_options *o)
>
> void init_merge_options(struct merge_options *o)
> {
> + const char *merge_verbosity;
> memset(o, 0, sizeof(struct merge_options));
> o->verbosity = 2;
> o->buffer_output = 1;
> @@ -2171,9 +2172,9 @@ void init_merge_options(struct merge_options *o)
> o->renormalize = 0;
> o->detect_rename = 1;
> merge_recursive_config(o);
> - if (getenv("GIT_MERGE_VERBOSITY"))
> - o->verbosity =
> - strtol(getenv("GIT_MERGE_VERBOSITY"), NULL, 10);
> + merge_verbosity = getenv("GIT_MERGE_VERBOSITY");
> + if (merge_verbosity)
> + o->verbosity = strtol(merge_verbosity, NULL, 10);
> if (o->verbosity >= 5)
> o->buffer_output = 0;
> strbuf_init(&o->obuf, 0);
