Skip to content

Commit 1048aa8

Browse files
committed
safe.directory: preliminary clean-up
The paths given in the safe.directory configuration variable are allowed to contain "~user" (which interpolates to user's home directory) and "%(prefix)" (which interpolates to the installation location in RUNTIME_PREFIX-enabled builds, and a call to the git_config_pathname() function is tasked to obtain a copy of the path with these constructs interpolated. The function, when it succeeds, always yields an allocated string in the location given as the out-parameter; even when there is nothing to interpolate in the original, a literal copy is made. The code path that contains this caller somehow made two contradicting and incorrect assumptions of the behaviour when there is no need for interpolation, and was written with extra defensiveness against two phantom risks that do not exist. One wrong assumption was that the function might yield NULL when there is no interpolation. This led to the use of an extra "check" variable, conditionally holding either the interpolated or the original string. The assumption was with us since 8959555 (setup_git_directory(): add an owner check for the top-level directory, 2022-03-02) originally introduced the safe.directory feature. Another wrong assumption was that the function might yield the same pointer as the input when there is no interpolation. This led to a conditional free'ing of the interpolated copy, that the conditional never skipped, as we always received an allocated string. Simplify the code by removing the extra defensiveness. Signed-off-by: Junio C Hamano <[email protected]>
1 parent d19b6cd commit 1048aa8

File tree

1 file changed

+5
-7
lines changed

1 file changed

+5
-7
lines changed

setup.c

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1235,17 +1235,15 @@ static int safe_directory_cb(const char *key, const char *value,
12351235
char *allowed = NULL;
12361236

12371237
if (!git_config_pathname(&allowed, key, value)) {
1238-
const char *check = allowed ? allowed : value;
1239-
if (ends_with(check, "/*")) {
1240-
size_t len = strlen(check);
1241-
if (!fspathncmp(check, data->path, len - 1))
1238+
if (ends_with(allowed, "/*")) {
1239+
size_t len = strlen(allowed);
1240+
if (!fspathncmp(allowed, data->path, len - 1))
12421241
data->is_safe = 1;
1243-
} else if (!fspathcmp(data->path, check)) {
1242+
} else if (!fspathcmp(data->path, allowed)) {
12441243
data->is_safe = 1;
12451244
}
1246-
}
1247-
if (allowed != value)
12481245
free(allowed);
1246+
}
12491247
}
12501248

12511249
return 0;

0 commit comments

Comments
 (0)