summaryrefslogtreecommitdiffstats
path: root/config.c (follow)
Commit message (Collapse)AuthorAgeFilesLines
* Merge branch 'ps/build-sign-compare'Junio C Hamano2024-12-231-0/+1
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Start working to make the codebase buildable with -Wsign-compare. * ps/build-sign-compare: t/helper: don't depend on implicit wraparound scalar: address -Wsign-compare warnings builtin/patch-id: fix type of `get_one_patchid()` builtin/blame: fix type of `length` variable when emitting object ID gpg-interface: address -Wsign-comparison warnings daemon: fix type of `max_connections` daemon: fix loops that have mismatching integer types global: trivial conversions to fix `-Wsign-compare` warnings pkt-line: fix -Wsign-compare warning on 32 bit platform csum-file: fix -Wsign-compare warning on 32-bit platform diff.h: fix index used to loop through unsigned integer config.mak.dev: drop `-Wno-sign-compare` global: mark code units that generate warnings with `-Wsign-compare` compat/win32: fix -Wsign-compare warning in "wWinMain()" compat/regex: explicitly ignore "-Wsign-compare" warnings git-compat-util: introduce macros to disable "-Wsign-compare" warnings
| * global: mark code units that generate warnings with `-Wsign-compare`Patrick Steinhardt2024-12-061-0/+1
| | | | | | | | | | | | | | | | | | Mark code units that generate warnings with `-Wsign-compare`. This allows for a structured approach to get rid of all such warnings over time in a way that can be easily measured. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | config: make `packed_git_(limit|window_size)` non-global variablesKarthik Nayak2024-12-041-17/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The variables `packed_git_window_size` and `packed_git_limit` are global config variables used in the `packfile.c` file. Since it is only used in this file, let's change it from being a global config variable to a local variable for the subsystem. With this, we rid `packfile.c` from all global variable usage and this means we can also remove the `USE_THE_REPOSITORY_VARIABLE` guard from the file. Helped-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | config: make `delta_base_cache_limit` a non-global variableKarthik Nayak2024-12-041-5/+0
|/ | | | | | | | | | | | | | | | | | | | | | | | | | The `delta_base_cache_limit` variable is a global config variable used by multiple subsystems. Let's make this non-global, by adding this variable independently to the subsystems where it is used. First, add the setting to the `repo_settings` struct, this provides access to the config in places where the repository is available. Use this in `packfile.c`. In `index-pack.c` we add it to the `pack_idx_option` struct and its constructor. While the repository struct is available here, it may not be set because `git index-pack` can be used without a repository. In `gc.c` add it to the `gc_config` struct and also the constructor function. The gc functions currently do not have direct access to a repository struct. These changes are made to remove the usage of `delta_base_cache_limit` as a global variable in `packfile.c`. This brings us one step closer to removing the `USE_THE_REPOSITORY_VARIABLE` definition in `packfile.c` which we complete in the next patch. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* config: fix evaluating "onbranch" with nonexistent git dirPatrick Steinhardt2024-09-241-6/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The `include_by_branch()` function is responsible for evaluating whether or not a specific include should be pulled in based on the currently checked out branch. Naturally, his condition can only be evaluated when we have a properly initialized repository with a ref store in the first place. This is why the function guards against the case when either `data->repo` or `data->repo->gitdir` are `NULL` pointers. But the second check is insufficient: the `gitdir` may be set even though the repository has not been initialized. Quoting "setup.c": NEEDSWORK: currently we allow bogus GIT_DIR values to be set in some code paths so we also need to explicitly setup the environment if the user has set GIT_DIR. It may be beneficial to disallow bogus GIT_DIR values at some point in the future. So when either the GIT_DIR environment variable or the `--git-dir` global option are set by the user then `the_repository` may end up with an initialized `gitdir` variable. And this happens even when the dir is invalid, like for example when it doesn't exist. It follows that only checking for whether or not `gitdir` is `NULL` is not sufficient for us to determine whether the repository has been properly initialized. This issue can lead to us triggering a BUG: when using a config with an "includeIf.onbranch:" condition outside of a repository while using the `--git-dir` option pointing to an invalid Git directory we may end up trying to evaluate the condition even though the ref storage format has not been set up. This bisects to 173761e21b (setup: start tracking ref storage format, 2023-12-29), but that commit really only starts to surface the issue that has already existed beforehand. The code to check for `gitdir` was introduced via 85fe0e800c (config: work around bug with includeif:onbranch and early config, 2019-07-31), which tried to fix similar issues when we didn't yet have a repository set up. But the fix was incomplete as it missed the described scenario. As the quoted comment mentions, we'd ideally refactor the code to not set up `gitdir` with an invalid value in the first place, but that may be a bigger undertaking. Instead, refactor the code to use the ref storage format as an indicator of whether or not the ref store has been set up to fix the bug. Reported-by: Ronan Pigott <ronan@rjp.ie> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* environment: stop storing "core.notesRef" globallyPatrick Steinhardt2024-09-121-8/+0
| | | | | | | | | | Stop storing the "core.notesRef" config value globally. Instead, retrieve the value in `default_notes_ref()`. The code is never called in a hot loop anyway, so doing this on every invocation should be perfectly fine. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* environment: stop storing "core.warnAmbiguousRefs" globallyPatrick Steinhardt2024-09-121-5/+0
| | | | | | | | | | | | | | | | | | | Same as the preceding commits, storing the "core.warnAmbiguousRefs" value globally is misdesigned as this setting may be set per repository. Move the logic into the repo-settings subsystem. The usual pattern here is that users are expected to call `prepare_repo_settings()` before they access the settings themselves. This seems somewhat fragile though, as it is easy to miss and leads to somewhat ugly code patterns at the call sites. Instead, introduce a new function that encapsulates this logic for us. This also allows us to change how exactly the lazy initialization works in the future, e.g. by only partially initializing values as requested by the caller. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* environment: stop storing "core.preferSymlinkRefs" globallyPatrick Steinhardt2024-09-121-5/+0
| | | | | | | | | | | | Same as the preceding commit, storing the "core.preferSymlinkRefs" value globally is misdesigned as this setting may be set per repository. There is only a single user of this value anyway, namely the "files" backend. So let's just remove the global variable and read the value of this setting when initializing the backend. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* environment: stop storing "core.logAllRefUpdates" globallyPatrick Steinhardt2024-09-121-10/+0
| | | | | | | | | | | | | | | | | The value of "core.logAllRefUpdates" is being stored in the global variable `log_all_ref_updates`. This design is somewhat aged nowadays, where it is entirely possible to access multiple repositories in the same process which all have different values for this setting. So using a single global variable to track it is plain wrong. Remove the global variable. Instead, we now provide a new function part of the repo-settings subsystem that parses the value for a specific repository. While that may require us to read the value multiple times, we work around this by reading it once when the ref backends are set up and caching the value there. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* environment: guard state depending on a repositoryPatrick Steinhardt2024-09-121-0/+2
| | | | | | | | | | | | | | | | | | | In "environment.h" we have quite a lot of functions and variables that either explicitly or implicitly depend on `the_repository`. The implicit set of stateful declarations includes for example variables which get populated when parsing a repository's Git configuration. This set of variables is broken by design, as their state often depends on the last repository config that has been parsed. So they may or may not represent the state of `the_repository`. Fixing that is quite a big undertaking, and later patches in this series will demonstrate a solution for a first small set of those variables. So for now, let's guard these with `USE_THE_REPOSITORY_VARIABLE` so that callers are aware of the implicit dependency. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* config: make dependency on repo in `read_early_config()` explicitPatrick Steinhardt2024-09-121-6/+4
| | | | | | | | | | | | | | | | The `read_early_config()` function can be used to read configuration where a repository has not yet been set up. As such, it is optional whether or not `the_repository` has already been initialized. If it was initialized we use its commondir and gitdir. If not, the function will try to detect the Git directories by itself and, if found, also parse their config files. This means that we implicitly rely on `the_repository`. Make this dependency explicit by passing a `struct repository`. This allows us to again drop the `USE_THE_REPOSITORY_VARIABLE` define in "config.c". Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* config: document `read_early_config()` and `read_very_early_config()`Patrick Steinhardt2024-09-121-4/+0
| | | | | | | | | | It's not clear what `read_early_config()` and `read_very_early_config()` do differently compared to `repo_read_config()` from just looking at their names. Document both of these in the header file to clarify their intent. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* environment: make `get_git_common_dir()` accept a repositoryPatrick Steinhardt2024-09-121-1/+1
| | | | | | | | | | | The `get_git_common_dir()` function retrieves the path to the common directory for `the_repository`. Make it accept a `struct repository` such that it can work on arbitrary repositories and make it part of the repository subsystem. This reduces our reliance on `the_repository` and clarifies scope. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* environment: make `get_git_dir()` accept a repositoryPatrick Steinhardt2024-09-121-1/+3
| | | | | | | | | | | The `get_git_dir()` function retrieves the path to the Git directory for `the_repository`. Make it accept a `struct repository` such that it can work on arbitrary repositories and make it part of the repository subsystem. This reduces our reliance on `the_repository` and clarifies scope. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'ps/maintenance-detach-fix'Junio C Hamano2024-08-261-2/+3
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | Maintenance tasks other than "gc" now properly go background when "git maintenance" runs them. * ps/maintenance-detach-fix: run-command: fix detaching when running auto maintenance builtin/maintenance: add a `--detach` flag builtin/gc: add a `--detach` flag builtin/gc: stop processing log file on signal builtin/gc: fix leaking config values builtin/gc: refactor to read config into structure config: fix constness of out parameter for `git_config_get_expiry()`
| * config: fix constness of out parameter for `git_config_get_expiry()`Patrick Steinhardt2024-08-161-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The type of the out parameter of `git_config_get_expiry()` is a pointer to a constant string, which creates the impression that ownership of the returned data wasn't transferred to the caller. This isn't true though and thus quite misleading. Adapt the parameter to be of type `char **` and adjust callers accordingly. While at it, refactor `get_shared_index_expire_date()` to drop the static `shared_index_expire` variable. It is only used in that function, and furthermore we would only hit the code where we parse the expiry date a single time because we already use a static `prepared` variable to track whether we did parse it. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'ps/config-wo-the-repository'Junio C Hamano2024-08-231-149/+76
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Use of API functions that implicitly depend on the_repository object in the config subsystem has been rewritten to pass a repository object through the callchain. * ps/config-wo-the-repository: config: hide functions using `the_repository` by default global: prepare for hiding away repo-less config functions config: don't depend on `the_repository` with branch conditions config: don't have setters depend on `the_repository` config: pass repo to functions that rename or copy sections config: pass repo to `git_die_config()` config: pass repo to `git_config_get_expiry_in_days()` config: pass repo to `git_config_get_expiry()` config: pass repo to `git_config_get_max_percent_split_change()` config: pass repo to `git_config_get_split_index()` config: pass repo to `git_config_get_index_threads()` config: expose `repo_config_clear()` config: introduce missing setters that take repo as parameter path: hide functions using `the_repository` by default path: stop relying on `the_repository` in `worktree_git_path()` path: stop relying on `the_repository` when reporting garbage hooks: remove implicit dependency on `the_repository` editor: do not rely on `the_repository` for interactive edits path: expose `do_git_common_path()` as `repo_common_pathv()` path: expose `do_git_path()` as `repo_git_pathv()`
| * | config: hide functions using `the_repository` by defaultPatrick Steinhardt2024-08-131-130/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The config subsystem provides a bunch of legacy functions that read or set configuration for `the_repository`. The use of those functions is discouraged, and it is easy to miss the implicit dependency on `the_repository` that calls to those functions may cause. Move all config-related functions that use `the_repository` into a block that gets only conditionally compiled depending on whether or not the macro has been defined. This also removes all dependencies on that variable in "config.c", allowing us to remove the definition of said preprocessor macro. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | config: don't depend on `the_repository` with branch conditionsPatrick Steinhardt2024-08-131-4/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When computing branch "includeIf" conditions we use `the_repository` to obtain the main ref store. We really shouldn't depend on this global repository though, but should instead use the repository that is being passed to us via `struct config_include_data`. Otherwise, when parsing configuration of e.g. submodules, we may end up evaluating the condition the via the wrong refdb. Fix this. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | config: don't have setters depend on `the_repository`Patrick Steinhardt2024-08-131-8/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Some of the setters that accept a `struct repository` still implicitly rely on `the_repository` via `git_config_set_multivar_in_file()`. While this function would typically use the caller-provided path, it knows to fall back to using the configuration path indicated by `the_repository`. Adapt those functions to instead use the caller-provided repository. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | config: pass repo to functions that rename or copy sectionsPatrick Steinhardt2024-08-131-14/+16
| | | | | | | | | | | | | | | | | | | | | | | | | | | Refactor functions that rename or copy config sections to accept a `struct repository` such that we can get rid of the implicit dependency on `the_repository`. Rename the functions accordingly. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | config: pass repo to `git_die_config()`Patrick Steinhardt2024-08-131-6/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | Refactor `git_die_config()` to accept a `struct repository` such that we can get rid of the implicit dependency on `the_repository`. Rename the function accordingly. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | config: pass repo to `git_config_get_expiry_in_days()`Patrick Steinhardt2024-08-131-2/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | Refactor `git_config_get_expiry_in_days()` to accept a `struct repository` such that we can get rid of the implicit dependency on `the_repository`. Rename the function accordingly. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | config: pass repo to `git_config_get_expiry()`Patrick Steinhardt2024-08-131-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | Refactor `git_config_get_expiry()` to accept a `struct repository` such that we can get rid of the implicit dependency on `the_repository`. Rename the function accordingly. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | config: pass repo to `git_config_get_max_percent_split_change()`Patrick Steinhardt2024-08-131-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | Refactor `git_config_get_max_percent_split_change()` to accept a `struct repository` such that we can get rid of the implicit dependency on `the_repository`. Rename the function accordingly. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | config: pass repo to `git_config_get_split_index()`Patrick Steinhardt2024-08-131-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | Refactor `git_config_get_split_index()` to accept a `struct repository` such that we can get rid of the implicit dependency on `the_repository`. Rename the function accordingly. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | config: pass repo to `git_config_get_index_threads()`Patrick Steinhardt2024-08-131-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | Refactor `git_config_get_index_threads()` to accept a `struct repository` such that we can get rid of the implicit dependency on `the_repository`. Rename the function accordingly. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | config: expose `repo_config_clear()`Patrick Steinhardt2024-08-131-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | While we already have `repo_config_clear()` as an alternative to `git_config_clear()` that doesn't rely on `the_repository`, it is not exposed to callers outside of the config subsystem. Do so. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | config: introduce missing setters that take repo as parameterPatrick Steinhardt2024-08-131-20/+73
| |/ | | | | | | | | | | | | | | | | | | While we already provide some of the config-setting interfaces with a `struct repository` as parameter, others only have a variant that implicitly depends on `the_repository`. Fill in those gaps such that we can start to deprecate the repo-less variants. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'ps/leakfixes-part-4'Junio C Hamano2024-08-231-1/+2
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | More leak fixes. * ps/leakfixes-part-4: (22 commits) builtin/diff: free symmetric diff members diff: free state populated via options builtin/log: fix leak when showing converted blob contents userdiff: fix leaking memory for configured diff drivers builtin/format-patch: fix various trivial memory leaks diff: fix leak when parsing invalid ignore regex option unpack-trees: clear index when not propagating it sequencer: release todo list on error paths merge-ort: unconditionally release attributes index builtin/fast-export: plug leaking tag names builtin/fast-export: fix leaking diff options builtin/fast-import: plug trivial memory leaks builtin/notes: fix leaking `struct notes_tree` when merging notes builtin/rebase: fix leaking `commit.gpgsign` value config: fix leaking comment character config submodule-config: fix leaking name entry when traversing submodules read-cache: fix leaking hashfile when writing index fails bulk-checkin: fix leaking state TODO object-name: fix leaking symlink paths in object context object-file: fix memory leak when reading corrupted headers ...
| * | config: fix leaking comment character configPatrick Steinhardt2024-08-141-1/+2
| |/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When the comment line character has been specified multiple times in the configuration, then `git_default_core_config()` will cause a memory leak because it unconditionally copies the string into `comment_line_str` without free'ing the previous value. In fact, it can't easily free the value in the first place because it may contain a string constant. Refactor the code such that we track allocated comment character strings via a separate non-constant variable `comment_line_str_to_free`. Adapt sites that set `comment_line_str` to set both and free the old value that was stored in `comment_line_str_to_free`. This memory leak is being hit in t3404. As there are still other memory leaks in that file we cannot yet mark it as passing with leak checking enabled. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'tb/config-fixed-value-with-valueless-true'Junio C Hamano2024-08-141-1/+1
|\ \ | |/ |/| | | | | | | | | | | | | "git config --value=foo --fixed-value section.key newvalue" barfed when the existing value in the configuration file used the valueless true syntax, which has been corrected. * tb/config-fixed-value-with-valueless-true: config.c: avoid segfault with --fixed-value and valueless config
| * config.c: avoid segfault with --fixed-value and valueless configTaylor Blau2024-08-011-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When using `--fixed-value` with a key whose value is left empty (implied as being "true"), 'git config' may crash when invoked like either of: $ git config set --file=config --value=value --fixed-value \ section.key pattern $ git config --file=config --fixed-value section.key value pattern The original bugreport[1] bisects to 00bbdde141 (builtin/config: introduce "set" subcommand, 2024-05-06), which is a red-herring, since the original bugreport uses the new 'git config set' invocation. The behavior likely bisects back to c90702a1f6 (config: plumb --fixed-value into config API, 2020-11-25), which introduces the new --fixed-value option in the first place. Looking at the relevant frame from a failed process's coredump, the crash appears in config.c::matches() like so: (gdb) up #1 0x000055b3e8b06022 in matches (key=0x55b3ea894360 "section.key", value=0x0, store=0x7ffe99076eb0) at config.c:2884 2884 return !strcmp(store->fixed_value, value); where we are trying to compare the `--fixed-value` argument to `value`, which is NULL. Avoid attempting to match `--fixed-value` for configuration keys with no explicit value. A future patch could consider the empty value to mean "true", "yes", "on", etc. when invoked with `--type=bool`, but let's punt on that for now in the name of avoiding the segfault. [1]: https://lore.kernel.org/git/CANrWfmTek1xErBLrnoyhHN+gWU+rw14y6SQ+abZyzGoaBjmiKA@mail.gmail.com/ Reported-by: Han Jiang <jhcarl0814@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'ps/leakfixes-more'Junio C Hamano2024-07-081-0/+1
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | More memory leaks have been plugged. * ps/leakfixes-more: (29 commits) builtin/blame: fix leaking ignore revs files builtin/blame: fix leaking prefixed paths blame: fix leaking data for blame scoreboards line-range: plug leaking find functions merge: fix leaking merge bases builtin/merge: fix leaking `struct cmdnames` in `get_strategy()` sequencer: fix memory leaks in `make_script_with_merges()` builtin/clone: plug leaking HEAD ref in `wanted_peer_refs()` apply: fix leaking string in `match_fragment()` sequencer: fix leaking string buffer in `commit_staged_changes()` commit: fix leaking parents when calling `commit_tree_extended()` config: fix leaking "core.notesref" variable rerere: fix various trivial leaks builtin/stash: fix leak in `show_stash()` revision: free diff options builtin/log: fix leaking commit list in git-cherry(1) merge-recursive: fix memory leak when finalizing merge builtin/merge-recursive: fix leaking object ID bases builtin/difftool: plug memory leaks in `run_dir_diff()` object-name: free leaking object contexts ...
| * | config: fix leaking "core.notesref" variablePatrick Steinhardt2024-06-111-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | The variable used to track the "core.notesref" config is not getting freed before we assign to it and thus leaks. Fix this. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | Merge branch 'ps/leakfixes' into ps/leakfixes-moreJunio C Hamano2024-06-031-17/+38
| |\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * ps/leakfixes: builtin/mv: fix leaks for submodule gitfile paths builtin/mv: refactor to use `struct strvec` builtin/mv duplicate string list memory builtin/mv: refactor `add_slash()` to always return allocated strings strvec: add functions to replace and remove strings submodule: fix leaking memory for submodule entries commit-reach: fix memory leak in `ahead_behind()` builtin/credential: clear credential before exit config: plug various memory leaks config: clarify memory ownership in `git_config_string()` builtin/log: stop using globals for format config builtin/log: stop using globals for log config convert: refactor code to clarify ownership of check_roundtrip_encoding diff: refactor code to clarify memory ownership of prefixes config: clarify memory ownership in `git_config_pathname()` http: refactor code to clarify memory ownership checkout: clarify memory ownership in `unique_tracking_name()` strbuf: fix leak when `appendwholeline()` fails with EOF transport-helper: fix leaking helper name
* | \ \ Merge branch 'ps/use-the-repository'Junio C Hamano2024-07-021-0/+3
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A CPP macro USE_THE_REPOSITORY_VARIABLE is introduced to help transition the codebase to rely less on the availability of the singleton the_repository instance. * ps/use-the-repository: hex: guard declarations with `USE_THE_REPOSITORY_VARIABLE` t/helper: remove dependency on `the_repository` in "proc-receive" t/helper: fix segfault in "oid-array" command without repository t/helper: use correct object hash in partial-clone helper compat/fsmonitor: fix socket path in networked SHA256 repos replace-object: use hash algorithm from passed-in repository protocol-caps: use hash algorithm from passed-in repository oidset: pass hash algorithm when parsing file http-fetch: don't crash when parsing packfile without a repo hash-ll: merge with "hash.h" refs: avoid include cycle with "repository.h" global: introduce `USE_THE_REPOSITORY_VARIABLE` macro hash: require hash algorithm in `empty_tree_oid_hex()` hash: require hash algorithm in `is_empty_{blob,tree}_oid()` hash: make `is_null_oid()` independent of `the_repository` hash: convert `oidcmp()` and `oideq()` to compare whole hash global: ensure that object IDs are always padded hash: require hash algorithm in `oidread()` and `oidclr()` hash: require hash algorithm in `hasheq()`, `hashcmp()` and `hashclr()` hash: drop (mostly) unused `is_empty_{blob,tree}_sha1()` functions
| * | | | global: introduce `USE_THE_REPOSITORY_VARIABLE` macroPatrick Steinhardt2024-06-141-0/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Use of the `the_repository` variable is deprecated nowadays, and we slowly but steadily convert the codebase to not use it anymore. Instead, callers should be passing down the repository to work on via parameters. It is hard though to prove that a given code unit does not use this variable anymore. The most trivial case, merely demonstrating that there is no direct use of `the_repository`, is already a bit of a pain during code reviews as the reviewer needs to manually verify claims made by the patch author. The bigger problem though is that we have many interfaces that implicitly rely on `the_repository`. Introduce a new `USE_THE_REPOSITORY_VARIABLE` macro that allows code units to opt into usage of `the_repository`. The intent of this macro is to demonstrate that a certain code unit does not use this variable anymore, and to keep it from new dependencies on it in future changes, be it explicit or implicit For now, the macro only guards `the_repository` itself as well as `the_hash_algo`. There are many more known interfaces where we have an implicit dependency on `the_repository`, but those are not guarded at the current point in time. Over time though, we should start to add guards as required (or even better, just remove them). Define the macro as required in our code units. As expected, most of our code still relies on the global variable. Nearly all of our builtins rely on the variable as there is no way yet to pass `the_repository` to their entry point. For now, declare the macro in "biultin.h" to keep the required changes at least a little bit more contained. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | Merge branch 'tb/pseudo-merge-reachability-bitmap'Junio C Hamano2024-06-251-0/+9
|\ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The pseudo-merge reachability bitmap to help more efficient storage of the reachability bitmap in a repository with too many refs has been added. * tb/pseudo-merge-reachability-bitmap: (26 commits) pack-bitmap.c: ensure pseudo-merge offset reads are bounded Documentation/technical/bitmap-format.txt: add missing position table t/perf: implement performance tests for pseudo-merge bitmaps pseudo-merge: implement support for finding existing merges ewah: `bitmap_equals_ewah()` pack-bitmap: extra trace2 information pack-bitmap.c: use pseudo-merges during traversal t/test-lib-functions.sh: support `--notick` in `test_commit_bulk()` pack-bitmap: implement test helpers for pseudo-merge ewah: implement `ewah_bitmap_popcount()` pseudo-merge: implement support for reading pseudo-merge commits pack-bitmap.c: read pseudo-merge extension pseudo-merge: scaffolding for reads pack-bitmap: extract `read_bitmap()` function pack-bitmap-write.c: write pseudo-merge table pseudo-merge: implement support for selecting pseudo-merge commits config: introduce `git_config_double()` pack-bitmap: make `bitmap_writer_push_bitmapped_commit()` public pack-bitmap: implement `bitmap_writer_has_bitmapped_object_id()` pack-bitmap-write: support storing pseudo-merge commits ...
| * | | | | config: introduce `git_config_double()`Taylor Blau2024-05-241-0/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Future commits will want to parse a double-precision floating point value from configuration, but we have no way to parse such a value prior to this patch. The core of the routine is implemented in git_parse_double(). Unlike git_parse_unsigned() and git_parse_signed(), however, the function implemented here only works on type "double", and not related types like "float", or "long double". This is because "float" and "long double" use different functions to convert from ASCII strings to floating point values (strtof() and strtold(), respectively). Likewise, there is no pointer type that can assign to any of these values (except for "void *"), so the only way to define this trio of functions would be with a macro expansion that is parameterized over the floating point type and conversion function. That is all doable, but likely to be overkill given our current needs, which is only to parse double-precision floats. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | Merge branch 'ps/abbrev-length-before-setup-fix'Junio C Hamano2024-06-211-2/+2
|\ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Setting core.abbrev too early before the repository set-up (typically in "git clone") caused segfault, which as been corrected. * ps/abbrev-length-before-setup-fix: object-name: don't try to abbreviate to lengths greater than hexsz parse-options-cb: stop clamping "--abbrev=" to hash length config: fix segfault when parsing "core.abbrev" without repo
| * | | | | | config: fix segfault when parsing "core.abbrev" without repoPatrick Steinhardt2024-06-121-2/+2
| | |_|/ / / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The "core.abbrev" config allows the user to specify the minimum length when abbreviating object hashes. Next to the values "auto" and "no", this config also accepts a concrete length that needs to be bigger or equal to the minimum length and smaller or equal to the hash algorithm's hex length. While the former condition is trivial, the latter depends on the object format used by the current repository. It is thus a variable upper boundary that may either be 40 (SHA-1) or 64 (SHA-256). This has two major downsides. First, the user that specifies this config must be aware of the object hashes that its repository use. If they want to configure the value globally, then they cannot pick any value in the range `[41, 64]` if they have any repository that uses SHA-1. If they did, Git would error out when parsing the config. Second, and more importantly, parsing "core.abbrev" crashes when outside of a Git repository because we dereference `the_hash_algo` to figure out its hex length. Starting with c8aed5e8da (repository: stop setting SHA1 as the default object hash, 2024-05-07) though, we stopped initializing `the_hash_algo` outside of Git repositories. Fix both of these issues by not making it an error anymore when the given length exceeds the hash length. Instead, leave the abbreviated length intact. `repo_find_unique_abbrev_r()` handles this just fine except for a performance penalty which we will fix in a subsequent commit. Reported-by: Kyle Lippincott <spectral@google.com> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | Merge branch 'jc/varargs-attributes'Junio C Hamano2024-06-181-1/+0
|\ \ \ \ \ \ | |_|_|/ / / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Varargs functions that are unannotated as printf-like or execl-like have been annotated as such. * jc/varargs-attributes: __attribute__: add a few missing format attributes __attribute__: mark some functions with LAST_ARG_MUST_BE_NULL __attribute__: remove redundant attribute declaration for git_die_config() __attribute__: trace2_region_enter_printf() is like "printf"
| * | | | | __attribute__: remove redundant attribute declaration for git_die_config()Junio C Hamano2024-06-101-1/+0
| | |/ / / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The convention is to declare the function attribute to an extern function together with its declaration in the header file, without repeating the attribute declaration with its definition in the .c source file (a file-scope static function declares its attribute together with its definition in the .c file it is defined, as there is no other place to do so). The definition of git_die_config() in config.c did not follow the convention and had its attribute declared with both its declaration in the header and its definition in the .c source file. Remove the one in the config.c to match everybody else. Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | Merge branch 'ps/leakfixes'Junio C Hamano2024-06-061-17/+38
|\ \ \ \ \ | |_|/ / / |/| | / / | | |/ / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Leakfixes. * ps/leakfixes: builtin/mv: fix leaks for submodule gitfile paths builtin/mv: refactor to use `struct strvec` builtin/mv duplicate string list memory builtin/mv: refactor `add_slash()` to always return allocated strings strvec: add functions to replace and remove strings submodule: fix leaking memory for submodule entries commit-reach: fix memory leak in `ahead_behind()` builtin/credential: clear credential before exit config: plug various memory leaks config: clarify memory ownership in `git_config_string()` builtin/log: stop using globals for format config builtin/log: stop using globals for log config convert: refactor code to clarify ownership of check_roundtrip_encoding diff: refactor code to clarify memory ownership of prefixes config: clarify memory ownership in `git_config_pathname()` http: refactor code to clarify memory ownership checkout: clarify memory ownership in `unique_tracking_name()` strbuf: fix leak when `appendwholeline()` fails with EOF transport-helper: fix leaking helper name
| * | | config: plug various memory leaksPatrick Steinhardt2024-05-271-9/+27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Now that memory ownership rules around `git_config_string()` and `git_config_pathname()` are clearer, it also got easier to spot that the returned memory needs to be free'd. Plug a subset of those cases and mark now-passing tests as leak free. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | config: clarify memory ownership in `git_config_string()`Patrick Steinhardt2024-05-271-3/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The out parameter of `git_config_string()` is a `const char **` even though we transfer ownership of memory to the caller. This is quite misleading and has led to many memory leaks all over the place. Adapt the parameter to instead be `char **`. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | convert: refactor code to clarify ownership of check_roundtrip_encodingPatrick Steinhardt2024-05-271-2/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The `check_roundtrip_encoding` variable is tracked in a `const char *` even though it may contain allocated strings at times. The result is that those strings may be leaking because we never free them. Refactor the code to always store allocated strings in this variable. The default value is handled in `check_roundtrip()` now, which is the only user of the variable. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | config: clarify memory ownership in `git_config_pathname()`Patrick Steinhardt2024-05-271-5/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The out parameter of `git_config_pathname()` is a `const char **` even though we transfer ownership of memory to the caller. This is quite misleading and has led to many memory leaks all over the place. Adapt the parameter to instead be `char **`. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | Merge branch 'jc/fix-2.45.1-and-friends-for-maint'Junio C Hamano2024-05-301-12/+1
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Adjust jc/fix-2.45.1-and-friends-for-2.39 for more recent maintenance track. * jc/fix-2.45.1-and-friends-for-maint: Revert "fsck: warn about symlink pointing inside a gitdir" Revert "Add a helper function to compare file contents" clone: drop the protections where hooks aren't run tests: verify that `clone -c core.hooksPath=/dev/null` works again Revert "core.hooksPath: add some protection while cloning" init: use the correct path of the templates directory again hook: plug a new memory leak ci: stop installing "gcc-13" for osx-gcc ci: avoid bare "gcc" for osx-gcc job ci: drop mention of BREW_INSTALL_PACKAGES variable send-email: avoid creating more than one Term::ReadLine object send-email: drop FakeTerm hack