summaryrefslogtreecommitdiffstats
AgeCommit message (Collapse)AuthorLines
2024-06-11revision: fix memory leak when reversing revisionsPatrick Steinhardt-0/+2
When reversing revisions in a rev walk, `get_revision()` will allocate a new commit list and assign it to `revs->commits`. It does not free the old list though, which makes it leak. Fix this. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-10The twelfth batchJunio C Hamano-0/+4
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-10Merge branch 'jk/leakfixes'Junio C Hamano-24/+29
Memory leaks in "git mv" has been plugged. * jk/leakfixes: mv: replace src_dir with a strvec mv: factor out empty src_dir removal mv: move src_dir cleanup to end of cmd_mv() t-strvec: mark variable-arg helper with LAST_ARG_MUST_BE_NULL t-strvec: use va_end() to match va_start()
2024-06-10Merge branch 'iw/trace-argv-on-alias'Junio C Hamano-5/+34
The alias-expanded command lines are logged to the trace output. * iw/trace-argv-on-alias: run-command: show prepared command Documentation: alias: add notes on shell expansion Documentation: alias: rework notes into points
2024-06-10diff: let external diffs report that changes are uninterestingRené Scharfe-12/+101
The options --exit-code and --quiet instruct git diff to indicate whether it found any significant changes by exiting with code 1 if it did and 0 if there were none. Currently this doesn't work if external diff programs are involved, as we have no way to learn what they found. Add that ability in the form of the new configuration options diff.trustExitCode and diff.<driver>.trustExitCode and the environment variable GIT_EXTERNAL_DIFF_TRUST_EXIT_CODE. They pair with the config options diff.external and diff.<driver>.command and the environment variable GIT_EXTERNAL_DIFF, respectively. The new options are off by default, keeping the old behavior. Enabling them indicates that the external diff returns exit code 1 if it finds significant changes and 0 if it doesn't, like diff(1). The name of the new options is taken from the git difftool and mergetool options of similar purpose. (There they enable passing on the exit code of a diff tool and to infer whether a merge done by a merge tool is successful.) The new feature sets the diff flag diff_from_contents in diff_setup_done() if we need the exit code and are allowed to call external diffs. This disables the optimization that avoids calling the program with --quiet. Add it back by skipping the call if the external diff is not able to report empty diffs. We can only do that check after evaluating the file-specific attributes in run_external_diff(). If we do run the external diff with --quiet, send its output to /dev/null. I considered checking the output of the external diff to check whether its empty. It was added as 11be65cfa4 (diff: fix --exit-code with external diff, 2024-05-05) and quickly reverted, as it does not work with external diffs that do not write to stdout. There's no reason why a graphical diff tool would even need to write anything there at all. I also considered using a non-zero exit code for empty diffs, which could be done without adding new configuration options. We'd need to disable the optimization that allows git diff --quiet to skip calling external diffs, though -- that might be quite surprising if graphical diff programs are involved. And assigning the opposite meaning of the exit codes compared to diff(1) and git diff --exit-code to the external diff can cause unnecessary confusion. Suggested-by: Phillip Wood <phillip.wood123@gmail.com> Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-10userdiff: add and use struct external_diffRené Scharfe-18/+24
Wrap the string specifying the external diff command in a new struct to simplify adding attributes, which the next patch will do. Make sure external_diff() still returns NULL if neither the environment variable GIT_EXTERNAL_DIFF nor the configuration option diff.external is set, to continue allowing its use in a boolean context. Use a designated initializer for the default builtin userdiff driver to adjust to the type change of the second struct member. Spelling out only the non-zero members improves readability as a nice side-effect. No functional change intended. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-10t4020: test exit code with external diffsRené Scharfe-0/+54
Add tests to check the exit code of git diff with its options --quiet and --exit-code when using an external diff program. Currently we cannot tell whether it found significant changes or not. While at it, document briefly that --quiet turns off execution of external diff programs because that behavior surprised me for a moment while writing the tests. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-10__attribute__: add a few missing format attributesJunio C Hamano-0/+3
A public function mem_pool_strfmt() takes printf like parameters, but is not given an attribute as such. Also a few file-scope static functions were missing their format attribute. Add them. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-10__attribute__: mark some functions with LAST_ARG_MUST_BE_NULLJunio C Hamano-0/+5
Some varargs functions that use NULL-terminated parameter list were missing __attributes__ ((sentinel)) aka LAST_ARG_MUST_BE_NULL. Add them. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-10__attribute__: remove redundant attribute declaration for git_die_config()Junio C Hamano-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>
2024-06-10__attribute__: trace2_region_enter_printf() is like "printf"Junio C Hamano-1/+3
The last part of the parameter list the function takes is like parameters to printf. Mark it as such. An existing call that formats a value of type size_t using "%d" was found by the compiler with the help with this annotation; fix it. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-08worktree_git_path(): move the declaration to path.hJunio C Hamano-8/+10
The definition of this function is in path.c but its declaration is in worktree.h, which is something unexpected. The function is explained as "Similar to git_path()"; declaring it next to where git_path() is declared would make more sense. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07doc: interactive.singleKey is disabled by defaultDragan Simic-4/+4
Make it clear that the interactive.singleKey configuration option is disabled by default, using rather subtle wording that avoids an emphasis on the actual default value. This should eliminate any associated doubts. While there, touch up the remaining wording of the description a bit. Signed-off-by: Dragan Simic <dsimic@manjaro.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07format-patch: assume --cover-letter for diff in multi-patch seriesRubén Justo-0/+31
When we deal with a multi-patch series in git-format-patch(1), if we see `--interdiff` or `--range-diff` but no `--cover-letter`, we return with an error, saying: fatal: --range-diff requires --cover-letter or single patch or: fatal: --interdiff requires --cover-letter or single patch This makes sense because the cover-letter is where we place the diff from the previous version. However, considering that `format-patch` generates a multi-patch as needed, let's adopt a similar "cover as necessary" approach when using `--interdiff` or `--range-diff`. Therefore, relax the requirement for an explicit `--cover-letter` in a multi-patch series when the user says `--iterdiff` or `--range-diff`. Still, if only to return the error, respect "format.coverLetter=no" and `--no-cover-letter`. Signed-off-by: Rubén Justo <rjusto@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07t4014: cleanups in a few testsRubén Justo-5/+5
Arrange things we are going to create to be removed at end, and then start creating them. That way, we will clean them up even if we fail after creating some but before the end of the command. Signed-off-by: Rubén Justo <rjusto@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07Merge branch 'tb/midx-write-cleanup'Junio C Hamano-91/+117
Code clean-up around writing the .midx files. * tb/midx-write-cleanup: pack-bitmap.c: reimplement `midx_bitmap_filename()` with helper midx: replace `get_midx_rev_filename()` with a generic helper midx-write.c: support reading an existing MIDX with `packs_to_include` midx-write.c: extract `fill_packs_from_midx()` midx-write.c: extract `should_include_pack()` midx-write.c: pass `start_pack` to `compute_sorted_entries()` midx-write.c: reduce argument count for `get_sorted_entries()` midx-write.c: tolerate `--preferred-pack` without bitmaps
2024-06-07imap-send: free all_msgs strbuf in "out" labelJeff King-1/+1
We read stdin into a strbuf, but most code paths never release it, causing a leak (albeit a minor one, as we leak only when exiting from the main function of the program). Commit 56f4f4a29d (imap-send: minimum leakfix, 2024-06-04) did the minimum to plug the one instance we see in the test suite, when we read an empty input. But it was sufficient only because aside from this noop invocation, we don't test imap-send at all! The right spot to free is in the "out" label, which is hit by all code paths before leaving the function. We couldn't do that in 56f4f4a29d because there was no unified exit path. That came separately in 3aca5f7fb0 (imap-send: fix leaking memory in `imap_server_conf`, 2024-06-04), which cleaned up many other leaks (but not this one). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07Merge branch 'jc/t1517-more' into jk/imap-send-plug-all-msgs-leakJunio C Hamano-0/+53
* jc/t1517-more: imap-send: minimum leakfix t1517: more coverage for commands that work without repository
2024-06-07Merge branch 'ps/no-writable-strings' into jk/imap-send-plug-all-msgs-leakJunio C Hamano-935/+1564
* ps/no-writable-strings: (46 commits) config.mak.dev: enable `-Wwrite-strings` warning builtin/merge: always store allocated strings in `pull_twohead` builtin/rebase: always store allocated string in `options.strategy` builtin/rebase: do not assign default backend to non-constant field imap-send: fix leaking memory in `imap_server_conf` imap-send: drop global `imap_server_conf` variable mailmap: always store allocated strings in mailmap blob revision: always store allocated strings in output encoding remote-curl: avoid assigning string constant to non-const variable send-pack: always allocate receive status parse-options: cast long name for OPTION_ALIAS http: do not assign string constant to non-const field compat/win32: fix const-correctness with string constants pretty: add casts for decoration option pointers object-file: make `buf` parameter of `index_mem()` a constant object-file: mark cached object buffers as const ident: add casts for fallback name and GECOS entry: refactor how we remove items for delayed checkouts line-log: always allocate the output prefix line-log: stop assigning string constant to file parent buffer ...
2024-06-07config.mak.dev: enable `-Wwrite-strings` warningPatrick Steinhardt-0/+1
Writing to string constants is undefined behaviour and must be avoided in C. Even so, the compiler does not help us with this by default because those constants are not in fact marked as `const`. This makes it rather easy to accidentally assign a constant to a non-const variable or field and then later on try to either free it or write to it. Enable `-Wwrite-strings` to catch such mistakes. With this warning enabled, the type of string constants is changed to `const char[]` and will thus cause compiler warnings when being assigned to non-const fields and variables. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07builtin/merge: always store allocated strings in `pull_twohead`Patrick Steinhardt-7/+11
The `pull_twohead` configuration may sometimes contain an allocated string, and sometimes it may contain a string constant. Refactor this to instead always store an allocated string such that we can release its resources without risk. While at it, manage the lifetime of other config strings, as well. Note that we explicitly don't free `cleanup_arg` here. This is because the variable may be assigned a string constant via command line options. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07builtin/rebase: always store allocated string in `options.strategy`Patrick Steinhardt-7/+7
The `struct rebase_options::strategy` field is a `char *`, but we do end up assigning string constants to it in two cases: - When being passed a `--strategy=` option via the command line. - When being passed a strategy option via `--strategy-option=`, but not a strategy. This will cause warnings once we enable `-Wwrite-strings`. Ideally, we'd just convert the field to be a `const char *`. But we also assign to this field via the GIT_TEST_MERGE_ALGORITHM envvar, which we have to strdup(3P) into it. Instead, refactor the code to make sure that we only ever assign allocated strings to this field. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07builtin/rebase: do not assign default backend to non-constant fieldPatrick Steinhardt-9/+16
The `struct rebase_options::default_backend` field is a non-constant string, but is being assigned a constant via `REBASE_OPTIONS_INIT`. Fix this by using `xstrdup()` to assign the variable and introduce a new function `rebase_options_release()` that releases memory held by the structure, including the newly-allocated variable. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07imap-send: fix leaking memory in `imap_server_conf`Patrick Steinhardt-23/+42
We never free any of the config strings that we populate into the `struct imap_server_conf`. Fix this by creating a common exit path where we can free resources. While at it, drop the unused member `imap_server_conf::name`. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07imap-send: drop global `imap_server_conf` variablePatrick Steinhardt-27/+30
In "imap-send.c", we have a global `sturct imap_server_conf` variable that keeps track of the configuration of the IMAP server. This variable is being populated mostly via the Git configuration. Refactor the code to allocate the structure on the stack instead of having it globally. This change allows us to track its lifetime more closely. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07mailmap: always store allocated strings in mailmap blobPatrick Steinhardt-1/+1
Same as with the preceding commit, the `git_mailmap_blob` may sometimes contain an allocated string and sometimes it may contain a string constant. This is risky and can easily lead to bugs in case the variable is getting re-assigned, where the code may then try to free the previous value to avoid memory leaks. Safeguard the code by always storing allocated strings in the variable. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07revision: always store allocated strings in output encodingPatrick Steinhardt-1/+4
The `git_log_output_encoding` variable can be set via the `--encoding=` option. When doing so, we conditionally either assign it to the passed value, or if the value is "none" we assign it the empty string. Depending on which of the both code paths we pick though, the variable may end up being assigned either an allocated string or a string constant. This is somewhat risky and may easily lead to bugs when a different code path may want to reassign a new value to it, freeing the previous value. We already to this when parsing the "i18n.logoutputencoding" config in `git_default_i18n_config()`. But because the config is typically parsed before we parse command line options this has been fine so far. Regardless of that, safeguard the code such that the variable always contains an allocated string. While at it, also free the old value in case there was any to plug a potential memory leak. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07remote-curl: avoid assigning string constant to non-const variablePatrick Steinhardt-26/+27
When processing remote options, we split the option line into two by searching for a space. If there is one, we replace the space with '\0', otherwise we implicitly assume that the value is "true" and thus assign a string constant. As the return value of strchr(3P) weirdly enough is a `char *` even though it gets a `const char *` as input, the assigned-to variable also is a non-constant. This is fine though because the argument is in fact an allocated string, and thus we are allowed to modify it. But this will break once we enable `-Wwrite-strings`. Refactor the code stop splitting the fields with '\0' altogether. Instead, we can pass the length of the option name to `set_option()` and then use strncmp(3P) instead of strcmp(3P). Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07send-pack: always allocate receive statusPatrick Steinhardt-1/+3
In `receive_status()`, we record the reason why ref updates have been rejected by the remote via the `remote_status`. But while we allocate the assigned string when a reason was given, we assign a string constant when no reason was given. This has been working fine so far due to two reasons: - We don't ever free the refs in git-send-pack(1)' - Remotes always give a reason, at least as implemented by Git proper. Adapt the code to always allocate the receive status string and free the refs. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07parse-options: cast long name for OPTION_ALIASPatrick Steinhardt-1/+1
We assign the long name for OPTION_ALIAS options to a non-constant value field. We know that the variable will never be written to, but this will cause warnings once we enable `-Wwrite-strings`. Cast away the constness to be prepared for this change. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07http: do not assign string constant to non-const fieldPatrick Steinhardt-3/+2
In `write_accept_language()`, we put all acceptable languages into an array. While all entries in that array are allocated strings, the final entry in that array is a string constant. This is fine because we explicitly skip over the last entry when freeing the array, but will cause warnings once we enable `-Wwrite-strings`. Adapt the code to also allocate the final entry. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07compat/win32: fix const-correctness with string constantsPatrick Steinhardt-15/+31
Adjust various places in our Win32 compatibility layer where we are not assigning string constants to `const char *` variables. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07pretty: add casts for decoration option pointersPatrick Steinhardt-2/+2
The `struct decoration_options` have a prefix and suffix field which are both non-constant, but we assign a constant pointer to them. This is safe to do because we pass them to `format_decorations()`, which never modifies these pointers, and then immediately discard the structure. Add explicit casts to avoid compilation warnings with `-Wwrite-strings`. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07object-file: make `buf` parameter of `index_mem()` a constantPatrick Steinhardt-7/+7
The `buf` parameter of `index_mem()` is a non-constant string. This will break once we enable `-Wwrite-strings` because we also pass constants from at least one callsite. Adapt the parameter to be a constant. As we cannot free the buffer without casting now, this also requires us to move the lifetime of the nested buffer around. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07object-file: mark cached object buffers as constPatrick Steinhardt-3/+5
The buffers of cached objects are never modified, but are still stored as a non-constant pointer. This will cause a compiler warning once we enable the `-Wwrite-strings` compiler warning as we assign an empty constant string when initializing the static `empty_tree` cached object. Convert the field to be constant. This requires us to shuffle around the code a bit because we memcpy(3P) into the allocated buffer in `pretend_object_file()`. This is easily fixed though by allocating the buffer into a temporary variable first. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07ident: add casts for fallback name and GECOSPatrick Steinhardt-2/+2
In `xgetpwuid_self()`, we return a fallback identity when it was not possible to look up the current identity. This fallback identity needs to be internal and must never be written to by the calles as specified by getpwuid(3P). As both the `pw_name` and `pw_gecos` fields are marked as non-constant though, it will cause a warning to assign constant strings to them once compiling with `-Wwrite-strings`. Add explicit casts to avoid the warning. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07entry: refactor how we remove items for delayed checkoutsPatrick Steinhardt-4/+10
When finalizing a delayed checkout, we sort out several strings from the passed-in string list by first assigning the empty string to those filters and then calling `string_list_remove_empty_items()`. Assigning the empty string will cause compiler warnings though as the string is a `char *` once we enable `-Wwrite-strings`. Refactor the code to use a `NULL` pointer with `filter_string_list()` instead to avoid this warning. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07line-log: always allocate the output prefixPatrick Steinhardt-7/+11
The returned string by `output_prefix()` is sometimes a string constant and sometimes an allocated string. This has been fine until now because we always leak the allocated strings, and thus we never tried to free the string constant. Fix the code to always return an allocated string and free the returned value at all callsites. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07line-log: stop assigning string constant to file parent bufferPatrick Steinhardt-1/+3
Stop assigning a string constant to the file parent buffer and instead assign an allocated string. While the code is fine in practice, it will break once we compile with `-Wwrite-strings`. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07diff: cast string constant in `fill_textconv()`Patrick Steinhardt-1/+1
The `fill_textconv()` function is responsible for converting an input file with a textconv driver, which is then passed to the caller. Weirdly though, the function also handles the case where there is no textconv driver at all. In that case, it will return either the contents of the populated filespec, or an empty string if the filespec is invalid. These two cases have differing memory ownership semantics. When there is a textconv driver, then the result is an allocated string. Otherwise, the result is either a string constant or owned by the filespec struct. All callers are in fact aware of this weirdness and only end up freeing the output buffer when they had a textconv driver. Ideally, we'd split up this interface to only perform the conversion via the textconv driver, and BUG in case the caller didn't provide one. This would make memory ownership semantics much more straight forward. For now though, let's simply cast the empty string constant to `char *` to avoid a warning with `-Wwrite-strings`. This is equivalent to the same cast that we already have in `fill_mmfile()`. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07builtin/remote: cast away constness in `get_head_names()`Patrick Steinhardt-6/+6
In `get_head_names()`, we assign the "refs/heads/*" string constant to `struct refspec_item::{src,dst}`, which are both non-constant pointers. Ideally, we'd refactor the code such that both of these fields were constant. But `struct refspec_item` is used for two different usecases with conflicting requirements: - To query for a source or destination based on the given refspec. The caller either sets `src` or `dst` as the branch that we want to search for, and the respective other field gets populated. The fields should be constant when being used as a query parameter, which is owned by the caller, and non-constant when being used as an out parameter, which is owned by the refspec item. This is is contradictory in itself already. - To store refspec items with their respective source and destination branches, in which case both fields should be owned by the struct. Ideally, we'd split up this interface to clearly separate between querying and storing, which would enable us to clarify lifetimes of the strings. This would be a much bigger undertaking though. Instead, accept the status quo for now and cast away the constness of the source and destination patterns. We know that those are not being written to or freed, so while this is ugly it certainly is fine for now. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07refspec: remove global tag refspec structurePatrick Steinhardt-19/+14
We have a global tag refspec structure that is used by both git-clone(1) and git-fetch(1). Initialization of the structure will break once we enable `-Wwrite-strings`, even though the breakage is harmless. While we could just add casts, the structure isn't really required in the first place as we can simply initialize the structures at the respective callsites. Refactor the code accordingly. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07reftable: cast away constness when assigning constants to recordsPatrick Steinhardt-63/+63
The reftable records are used in multiple ways throughout the reftable library. In many of those cases they merely act as input to a function without getting modified by it at all. Most importantly, this happens when writing records and when querying for records. We rely on this in our tests and thus assign string constants to those fields, which is about to generate warnings as those fields are of type `char *`. While we could go through the process and instead allocate those strings in all of our tests, this feels quite unnecessary. Instead, add casts to `char *` for all of those strings. As this is part of our tests, this also nicely serves as a demonstration that nothing writes or frees those string constants, which would otherwise lead to segfaults. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07refs/reftable: stop micro-optimizing refname allocations on copyPatrick Steinhardt-12/+16
When copying refs, we execute `write_copy_table()` to write the new table. As the names are given to us via `arg->newname` and `arg->oldname`, respectively, we optimize away some allocations by assigning those fields to the reftable records we are about to write directly, without duplicating them. This requires us to cast the input to `char *` pointers as they are in fact constant strings. Later on, we then unset the refname for all of the records before calling `reftable_log_record_release()` on them. We also do this when assigning the "HEAD" constant, but here we do not cast because its type is `char[]` by default. It's about to be turned into `const char *` though once we enable `-Wwrite-strings` and will thus cause another warning. It's quite dubious whether this micro-optimization really helps. We're about to write to disk anyway, which is going to be way slower than a small handful of allocations. Let's drop the optimization altogther and instead copy arguments to simplify the code and avoid the future warning with `-Wwrite-strings`. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07global: convert intentionally-leaking config strings to constsPatrick Steinhardt-16/+21
There are multiple cases where we intentionally leak config strings: - `struct gpg_format` is used to track programs that can be used for signing commits, either via gpg(1), gpgsm(1) or ssh-keygen(1). The user can override the commands via several config variables. As the array is populated once, only, and the struct memers are never written to or free'd. - `struct ll_merge_driver` is used to track merge drivers. Same as with the GPG format, these drivers are populated once and then reused. Its data is never written to or free'd, either. - `struct userdiff_funcname` and `struct userdiff_driver` can be configured via `diff.<driver>.*` to add additional drivers. Again, these have a global lifetime and are never written to or free'd. All of these are intentionally kept alive and are never written to. Furthermore, all of these are being assigned both string constants in some places, and allocated strings in other places. This will cause warnings once we enable `-Wwrite-strings`, so let's mark the respective fields as `const char *` and cast away the constness when assigning those values. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07global: improve const correctness when assigning string constantsPatrick Steinhardt-102/+106
We're about to enable `-Wwrite-strings`, which changes the type of string constants to `const char[]`. Fix various sites where we assign such constants to non-const variables. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07update-ref: add support for 'symref-update' commandKarthik Nayak-0/+306
Add 'symref-update' command to the '--stdin' mode of 'git-update-ref' to allow updates of symbolic refs. The 'symref-update' command takes in a <new-target>, which the <ref> will be updated to. If the <ref> doesn't exist it will be created. It also optionally takes either an `ref <old-target>` or `oid <old-oid>`. If the <old-target> is provided, it checks to see if the <ref> targets the <old-target> before the update. If <old-oid> is provided it checks <ref> to ensure that it is a regular ref and <old-oid> is the OID before the update. This by extension also means that this when a zero <old-oid> is provided, it ensures that the ref didn't exist before. The divergence in syntax from the regular `update` command is because if we don't use a `(ref | oid)` prefix for the old_value, then there is ambiguity around if the value provided should be treated as an oid or a reference. This is more so the reason, because we allow anything committish to be provided as an oid. While 'symref-verify' and 'symref-delete' also take in `<old-target>` we do not have this divergence there as those commands only work with symrefs. Whereas 'symref-update' also works with regular refs and allows users to convert regular refs to symrefs. The command allows users to perform symbolic ref updates within a transaction. This provides atomicity and allows users to perform a set of operations together. This command supports deref mode, to ensure that we can update dereferenced regular refs to symrefs. Helped-by: Patrick Steinhardt <ps@pks.im> Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07reftable: pick either 'oid' or 'target' for new updatesKarthik Nayak-2/+3
When creating a reference transaction update, we can provide the old/new oid/target for the update. We have checks in place to ensure that for each old/new, either oid or target is set and not both. In the reftable backend, when dealing with updates without the `REF_NO_DEREF` flag, we don't selectively propagate data as needed. Since there are no active users of the path, this is not caught. As we want to introduce the 'symref-update' command in the upcoming commit, which would use this flow, correct it. Helped-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07update-ref: add support for 'symref-create' commandKarthik Nayak-6/+146
Add 'symref-create' command to the '--stdin' mode 'git-update-ref' to allow creation of symbolic refs in a transaction. The 'symref-create' command takes in a <new-target>, which the created <ref> will point to. Also, support the 'core.prefersymlinkrefs' config, wherein if the config is set and the filesystem supports symlinks, we create the symbolic ref as a symlink. We fallback to creating a regular symref if creating the symlink is unsuccessful. Helped-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07update-ref: add support for 'symref-delete' commandKarthik Nayak-10/+140
Add a new command 'symref-delete' to allow deletions of symbolic refs in a transaction via the '--stdin' mode of the 'git-update-ref' command. The 'symref-delete' command can, when given an <old-target>, delete the provided <ref> only when it points to <old-target>. This command is only compatible with the 'no-deref' mode because we optionally want to check the 'old_target' of the ref being deleted. De-referencing a symbolic ref would provide a regular ref and we already have the 'delete' command for regular refs. While users can also use 'git symbolic-ref -d' to delete symbolic refs, the 'symref-delete' command in 'git-update-ref' allows users to do so within a transaction, which promises atomicity of the operation and can be batched with other commands. When no 'old_target' is provided it can also delete regular refs, similar to how the 'delete' command can delete symrefs when no 'old_oid' is provided. Helped-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>