Skip to content

Commit d4c4eac

Browse files
justinmksmjonas
authored andcommitted
refactor(log): simplify log_path_init neovim#18898
Problem: Since 22b52dd neovim#11501, log_path_init is called in log_init, so it is now called at a deterministic time. So the "just in time" complexity of log_path_init is no longer needed. Solution: Remove logic intended to try to "heal" partial initialization.
1 parent bbb19b8 commit d4c4eac

File tree

2 files changed

+21
-47
lines changed

2 files changed

+21
-47
lines changed

cmake/RunTests.cmake

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ set(ENV{NVIM_RPLUGIN_MANIFEST} ${BUILD_DIR}/Xtest_rplugin_manifest)
1111
set(ENV{XDG_CONFIG_HOME} ${BUILD_DIR}/Xtest_xdg/config)
1212
set(ENV{XDG_DATA_HOME} ${BUILD_DIR}/Xtest_xdg/share)
1313
unset(ENV{XDG_DATA_DIRS})
14+
unset(ENV{NVIM}) # Clear $NVIM in case tests are running from Nvim. #11009
1415

1516
if(NOT DEFINED ENV{NVIM_LOG_FILE})
1617
set(ENV{NVIM_LOG_FILE} ${BUILD_DIR}/.nvimlog)

src/nvim/log.c

Lines changed: 20 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -54,25 +54,19 @@ static bool log_try_create(char *fname)
5454

5555
/// Initializes path to log file. Sets $NVIM_LOG_FILE if empty.
5656
///
57-
/// Tries $NVIM_LOG_FILE, or falls back to $XDG_STATE_HOME/nvim/log. Path to log
58-
/// file is cached, so only the first call has effect, unless first call was not
59-
/// successful. Failed initialization indicates either a bug in expand_env()
60-
/// or both $NVIM_LOG_FILE and $HOME environment variables are undefined.
61-
///
62-
/// @return true if path was initialized, false otherwise.
63-
static bool log_path_init(void)
57+
/// Tries $NVIM_LOG_FILE, or falls back to $XDG_STATE_HOME/nvim/log. Failed
58+
/// initialization indicates either a bug in expand_env() or both $NVIM_LOG_FILE
59+
/// and $HOME environment variables are undefined.
60+
static void log_path_init(void)
6461
{
65-
if (log_file_path[0]) {
66-
return true;
67-
}
6862
size_t size = sizeof(log_file_path);
6963
expand_env((char_u *)"$" LOG_FILE_ENV, (char_u *)log_file_path,
7064
(int)size - 1);
7165
if (strequal("$" LOG_FILE_ENV, log_file_path)
7266
|| log_file_path[0] == '\0'
7367
|| os_isdir((char_u *)log_file_path)
7468
|| !log_try_create(log_file_path)) {
75-
// Make kXDGStateHome if it does not exist.
69+
// Make $XDG_STATE_HOME if it does not exist.
7670
char *loghome = get_xdg_home(kXDGStateHome);
7771
char *failed_dir = NULL;
7872
bool log_dir_failure = false;
@@ -91,7 +85,7 @@ static bool log_path_init(void)
9185
// Fall back to stderr
9286
if (len >= size || !log_try_create(log_file_path)) {
9387
log_file_path[0] = '\0';
94-
return false;
88+
return;
9589
}
9690
os_setenv(LOG_FILE_ENV, log_file_path, true);
9791
if (log_dir_failure) {
@@ -100,7 +94,6 @@ static bool log_path_init(void)
10094
}
10195
XFREE_CLEAR(failed_dir);
10296
}
103-
return true;
10497
}
10598

10699
void log_init(void)
@@ -166,21 +159,17 @@ bool logmsg(int log_level, const char *context, const char *func_name, int line_
166159
if (!did_msg) {
167160
did_msg = true;
168161
char *arg1 = func_name ? xstrdup(func_name) : (context ? xstrdup(context) : NULL);
162+
// coverity[leaked_storage]
169163
loop_schedule_deferred(&main_loop, event_create(on_log_recursive_event, 2, arg1, line_num));
170164
}
171165
g_stats.log_skip++;
172166
log_unlock();
173167
return false;
174168
}
175-
176169
recursive = true;
177170
bool ret = false;
178171
FILE *log_file = open_log_file();
179172

180-
if (log_file == NULL) {
181-
goto end;
182-
}
183-
184173
va_list args;
185174
va_start(args, fmt);
186175
ret = v_do_log_to_file(log_file, log_level, context, func_name, line_num,
@@ -190,7 +179,7 @@ bool logmsg(int log_level, const char *context, const char *func_name, int line_
190179
if (log_file != stderr && log_file != stdout) {
191180
fclose(log_file);
192181
}
193-
end:
182+
194183
recursive = false;
195184
log_unlock();
196185
return ret;
@@ -202,46 +191,36 @@ void log_uv_handles(void *loop)
202191
log_lock();
203192
FILE *log_file = open_log_file();
204193

205-
if (log_file == NULL) {
206-
goto end;
207-
}
208-
209194
uv_print_all_handles(l, log_file);
210195

211196
if (log_file != stderr && log_file != stdout) {
212197
fclose(log_file);
213198
}
214-
end:
199+
215200
log_unlock();
216201
}
217202

218203
/// Open the log file for appending.
219204
///
220-
/// @return FILE* decided by log_path_init() or stderr in case of error
205+
/// @return Log file, or stderr on failure
221206
FILE *open_log_file(void)
222207
{
223-
static bool recursive = false;
224-
if (recursive) {
225-
abort();
226-
}
227-
228-
FILE *log_file = NULL;
229-
recursive = true;
230-
if (log_path_init()) {
231-
log_file = fopen(log_file_path, "a");
232-
}
233-
recursive = false;
234-
235-
if (log_file != NULL) {
236-
return log_file;
208+
errno = 0;
209+
if (log_file_path[0]) {
210+
FILE *f = fopen(log_file_path, "a");
211+
if (f != NULL) {
212+
return f;
213+
}
237214
}
238215

239216
// May happen if:
240-
// - LOG() is called before early_init()
217+
// - fopen() failed
218+
// - LOG() is called before log_init()
241219
// - Directory does not exist
242220
// - File is not writable
243221
do_log_to_file(stderr, LOGLVL_ERR, NULL, __func__, __LINE__, true,
244-
"failed to open $" LOG_FILE_ENV ": %s", log_file_path);
222+
"failed to open $" LOG_FILE_ENV " (%s): %s",
223+
strerror(errno), log_file_path);
245224
return stderr;
246225
}
247226

@@ -285,13 +264,7 @@ void log_callstack(const char *const func_name, const int line_num)
285264
{
286265
log_lock();
287266
FILE *log_file = open_log_file();
288-
if (log_file == NULL) {
289-
goto end;
290-
}
291-
292267
log_callstack_to_file(log_file, func_name, line_num);
293-
294-
end:
295268
log_unlock();
296269
}
297270
#endif

0 commit comments

Comments
 (0)