On 7/31/2019 8:49 PM, Jeff King wrote:
On Wed, Jul 31, 2019 at 07:12:57PM -0400, Jeff King wrote:
Hrm. But common-main calls initialize_the_repository(), which points it
at &the_repo. And I can't find any other assignments. So how does it
become NULL? And is every caller of have_git_dir() at risk of
segfaulting?
Ah, I see. I think it is that trace2 reads the configuration very early.
I think we ought to do this:
diff --git a/common-main.c b/common-main.c
index 582a7b1886..89fd415e55 100644
--- a/common-main.c
+++ b/common-main.c
@@ -39,14 +39,14 @@ int main(int argc, const char **argv)
git_resolve_executable_dir(argv[0]);
+ initialize_the_repository();
+
trace2_initialize();
trace2_cmd_start(argv);
trace2_collect_process_info(TRACE2_PROCESS_INFO_STARTUP);
git_setup_gettext();
- initialize_the_repository();
-
attr_start();
result = cmd_main(argc, argv);
By the way, I wondered why trace2's existing config reading did not
cause us to segfault because of this. It is because it invented the
"very early config" function which always ignores some config sources
(working around this problem, but also making it weirdly unlike most
other config).
Yes, I added the "very early config" to try to work around some of
the chicken-n-egg problems. I can't say that I was completely happy
with having to do that. I haven't had time to play with your patch
suggestion here, but I think it would be fine to do if it will help
with the original problem.
In [1] I added code to just start the clock in isolation (rather than
being part of the trace2_initialize() -- which does all the config
loading and subsystem initialization). So it is OK to let the
trace2_initialize() run a little later. (Part of the reason for that
split was to allow git_resolve_executable_dir() to run first, since
that data was needed to find the location of the system config relative
to the exe path (sigh).)
[1] a089724958a trace2: refactor setting process starting time
So, as you suggested in your previous response, something like
this would/should be fine.
$ git diff
diff --git a/common-main.c b/common-main.c
index 582a7b1886..71e21dd20a 100644
--- a/common-main.c
+++ b/common-main.c
@@ -39,16 +39,16 @@ int main(int argc, const char **argv)
git_resolve_executable_dir(argv[0]);
- trace2_initialize();
- trace2_cmd_start(argv);
- trace2_collect_process_info(TRACE2_PROCESS_INFO_STARTUP);
-
git_setup_gettext();
initialize_the_repository();
attr_start();
+ trace2_initialize();
+ trace2_cmd_start(argv);
+ trace2_collect_process_info(TRACE2_PROCESS_INFO_STARTUP);
+
result = cmd_main(argc, argv);
trace2_cmd_exit(result);
