Hi. Thanks for looking at this.
Elijah Newren via GitGitGadget writes ("[PATCH] sequencer: honor GIT_REFLOG_ACTION"):
> I'm not the best with getenv/setenv. The xstrdup() wrapping is
> apparently necessary on mac and bsd. The xstrdup seems like it leaves us
> with a memory leak, but since setenv(3) says to not alter or free it, I
> think it's right. Anyone have any alternative suggestions?
I can try to help. It's not entirely trivial.
The setenv interface is a wrapper around putenv. putenv has had a
variety of different semantics. Some of these sets of semantics
cannot be used to re-set the same environment variable without a
memory leak - and even figuring out what semantics you have would be
complex and tend to produce code which would fail in bad ways.
There's a short summary of the situation in Linux's putenv(3).
Would it be possible for git to arrange to set GIT_REFLOG_ACTION only
when it is invoking subprocesses ? Otherwise it would update, and
look at, a global variable of its own. (Or a parameter to relevant
functions if one doesn't like the action-at-a-distance effect of a
global.)
And, it seems to me that the reflog handling should be centralised.
> + char *reflog_action = getenv("GIT_REFLOG_ACTION");
>
> va_start(ap, fmt);
> strbuf_reset(&buf);
> - strbuf_addstr(&buf, action_name(opts));
> + strbuf_addstr(&buf, reflog_action ? reflog_action : action_name(opts));
Open coding this kind of thing at every site which needs to think
about the reflog actions will surely result in some of the instances
having bugs.
Writing a single function that contans this (or most of it) would
happily decouple all of its call sites from literally asking about
getenv("GIT_REFLOG_ACTION") thereby making it easier to do the
indirection-through-program-variables I suggest.
Having said that,
> diff --git a/t/t3406-rebase-message.sh b/t/t3406-rebase-message.sh
> index 61b76f33019..927a4f4a4e4 100755
> --- a/t/t3406-rebase-message.sh
> +++ b/t/t3406-rebase-message.sh
This test case convinces me that the patch has the right behaviour for
at least the case I care about :-).
Thanks,
Ian.
--
Ian Jackson <ijackson@xxxxxxxxxxxxxxxxxxxxxx> These opinions are my own.
If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
a private address which bypasses my fierce spamfilter.
