git/apply.c, branch v2.46.2

Merge branch 'jk/apply-patch-mode-check-fix' into maint-2.46

2024-09-12T18:02:15Z

Test fix. * jk/apply-patch-mode-check-fix: t4129: fix racy index when calling chmod after git-add apply: canonicalize modes read from patches

apply: canonicalize modes read from patches

2024-08-05T15:50:43Z

Git stores only canonical modes for blobs. So for a regular file, we care about only "100644" or "100755" (depending only on the executable bit), but never modes where the group or other permissions are more exotic. So never "100664", "100700", etc. When a file in the working tree has such a mode, we quietly turn it into one of the two canonical modes, and that's what is stored both in the index and in tree objects. However, we don't canonicalize modes we read from incoming patches in git-apply. These may appear in a few lines: - "old mode" / "new mode" lines for mode changes - "new file mode" lines for newly created files - "deleted file mode" for removing files For "new mode" and for "new file mode", this is harmless. The patch is asking the result to have a certain mode, but: - when we add an index entry (for --index or --cached), it is canonicalized as we create the entry, via create_ce_mode(). - for a working tree file, try_create_file() passes either 0777 or 0666 to open(), so what you get depends only on your umask, not any other bits (aside from the executable bit) in the original mode. However, for "old mode" and "deleted file mode", there is a minor annoyance. We compare the patch's expected preimage mode with the current state. But that current state is always going to be a canonical mode itself: - updating an index entry via --cached will have the canonical mode in the index - for updating a working tree file, check_preimage() runs the mode through ce_mode_from_stat(), which does the usual canonicalization So if the patch feeds a non-canonical mode, it's impossible for it to match, and we will always complain with something like: file has type 100644, expected 100664 Since this is just a warning, the operation proceeds, but it's confusing and annoying. These cases should be pretty rare in practice. Git would never produce a patch with non-canonical modes itself (since it doesn't store them). And while we do accept patches from other programs, all of those lines were invented by Git. So you'd need a program trying to be Git compatible, but not handling canonicalization the same way. Reportedly "quilt" is such a program. We should canonicalize the modes as we read them so that the user never sees the useless warning. A few notes on the tests: - I've covered instances of all lines for completeness, even though the "new mode" / "new file mode" ones behave OK currently. - the tests apply patches to both the index and working tree, and check the result of both. Again, we know that all of these paths canonicalize anyway, but it's giving us extra coverage (although we are even less likely to have such a bug now since we canonicalize up front). - the test patches are missing "index" lines, which is also something Git would never produce. But they don't matter for the test, they do match the case from quilt we saw in the wild, and they avoid some sha1/sha256 complexity. Reported-by: Andrew Morton Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

Merge branch 'ps/leakfixes-more'

2024-07-08T21:53:10Z

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 ...

global: introduce `USE_THE_REPOSITORY_VARIABLE` macro

2024-06-14T17:26:33Z

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 Signed-off-by: Junio C Hamano

hash: require hash algorithm in `oidread()` and `oidclr()`

2024-06-14T17:26:32Z

Both `oidread()` and `oidclr()` use `the_repository` to derive the hash function that shall be used. Require callers to pass in the hash algorithm to get rid of this implicit dependency. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

apply: fix leaking string in `match_fragment()`

2024-06-11T20:15:07Z

Before calling `update_pre_post_images()`, we call `strbuf_detach()` to put its buffer into a new string variable that we then pass to that function. Besides being rather pointless, it also causes us to leak memory of that variable because we never free it. Get rid of the variable altogether and instead reach into the `strbuf` directly. While at it, refactor the code to have a common exit path and mark string that do not contain allocated memory as constant. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

parse-options: fix leaks for users of OPT_FILENAME

2024-06-11T20:15:04Z

The `OPT_FILENAME()` option will, if set, put an allocated string into the user-provided variable. Consequently, that variable thus needs to be free'd by the caller of `parse_options()`. Some callsites don't though and thus leak memory. Fix those. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

Merge branch 'rj/add-i-leak-fix'

2024-04-25T17:34:24Z

Leakfix. * rj/add-i-leak-fix: add: plug a leak on interactive_add add-patch: plug a leak handling the '/' command add-interactive: plug a leak in get_untracked_files apply: plug a leak in apply_data

apply: plug a leak in apply_data

2024-04-22T23:27:42Z

We have an execution path in apply_data that leaks the local struct image. Plug it. This leak can be triggered with: $ echo foo >file $ git add file && git commit -m file $ echo bar >file $ git diff file >diff $ sed s/foo/frotz/ baddiff $ git apply --cached Signed-off-by: Junio C Hamano

apply: avoid using fixed-size buffer in write_out_one_reject()

2024-04-16T20:38:35Z

On some systems PATH_MAX is not a hard limit. Support longer paths by building them on the heap instead of using static buffers. Take care to work around (arguably buggy) implementations of free(3) that change errno by calling it only after using the errno value. Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano