summaryrefslogtreecommitdiff
path: root/merge.c
AgeCommit message (Collapse)AuthorFilesLines
2020-08-18dir: fix problematic API to avoid memory leaksLibravatar Elijah Newren1-1/+2
The dir structure seemed to have a number of leaks and problems around it. First I noticed that parent_hashmap and recursive_hashmap were being leaked (though Peff noticed and submitted fixes before me). Then I noticed in the previous commit that clear_directory() was only taking responsibility for a subset of fields within dir_struct, despite the fact that entries[] and ignored[] we allocated internally to dir.c. That, of course, resulted in many callers either leaking or haphazardly trying to free these arrays and their contents. Digging further, I found that despite the pretty clear documentation near the top of dir.h that folks were supposed to call clear_directory() when the user no longer needed the dir_struct, there were four callers that didn't bother doing that at all. However, two of them clearly thought about leaks since they had an UNLEAK(dir) directive, which to me suggests that the method to free the data was too unclear. I suspect the non-obviousness of the API and its holes led folks to avoid it, which then snowballed into further problems with the entries[], ignored[], parent_hashmap, and recursive_hashmap problems. Rename clear_directory() to dir_clear() to be more in line with other data structures in git, and introduce a dir_init() to handle the suggested memsetting of dir_struct to all zeroes. I hope that a name like "dir_clear()" is more clear, and that the presence of dir_init() will provide a hint to those looking at the code that they need to look for either a dir_clear() or a dir_free() and lead them to find dir_clear(). Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-07-30strvec: rename struct fieldsLibravatar Jeff King1-1/+1
The "argc" and "argv" names made sense when the struct was argv_array, but now they're just confusing. Let's rename them to "nr" (which we use for counts elsewhere) and "v" (which is rather terse, but reads well when combined with typical variable names like "args.v"). Note that we have to update all of the callers immediately. Playing tricks with the preprocessor is hard here, because we wouldn't want to rewrite unrelated tokens. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-07-28strvec: convert remaining callers away from argv_array nameLibravatar Jeff King1-8/+8
We eventually want to drop the argv_array name and just use strvec consistently. There's no particular reason we have to do it all at once, or care about interactions between converted and unconverted bits. Because of our preprocessor compat layer, the names are interchangeable to the compiler (so even a definition and declaration using different names is OK). This patch converts all of the remaining files, as the resulting diff is reasonably sized. The conversion was done purely mechanically with: git ls-files '*.c' '*.h' | xargs perl -i -pe ' s/ARGV_ARRAY/STRVEC/g; s/argv_array/strvec/g; ' We'll deal with any indentation/style fallouts separately. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-03-16builtin/checkout: compute checkout metadata for checkoutsLibravatar brian m. carlson1-0/+1
Provide commit metadata for checkout code paths that use unpack_trees and friends. When we're checking out a commit, use the commit information, but don't provide commit information if we're checking out from the index, since there need not be any particular commit associated with the index, and even if there is one, we can't know what it is. Signed-off-by: brian m. carlson <bk2204@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-01-14read-cache.c: kill read_index()Libravatar Nguyễn Thái Ngọc Duy1-1/+1
read_index() shares the same problem as hold_locked_index(): it assumes $GIT_DIR/index. Move all call sites to repo_read_index() instead. read_index_preload() and read_index_unmerged() are also killed as a consequence. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-01-14repository.c: replace hold_locked_index() with repo_hold_locked_index()Libravatar Nguyễn Thái Ngọc Duy1-1/+1
hold_locked_index() assumes the index path at $GIT_DIR/index. This is not good for places that take an arbitrary index_state instead of the_index, which is basically everywhere except builtin/. Replace it with repo_hold_locked_index(). hold_locked_index() remains as a wrapper around repo_hold_locked_index() to reduce changes in builtin/ Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-09-21merge.c: remove implicit dependency on the_indexLibravatar Nguyễn Thái Ngọc Duy1-9/+11
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-02Merge branch 'en/dirty-merge-fixes'Libravatar Junio C Hamano1-31/+0
The recursive merge strategy did not properly ensure there was no change between HEAD and the index before performing its operation, which has been corrected. * en/dirty-merge-fixes: merge: fix misleading pre-merge check documentation merge-recursive: enforce rule that index matches head before merging t6044: add more testcases with staged changes before a merge is invoked merge-recursive: fix assumption that head tree being merged is HEAD merge-recursive: make sure when we say we abort that we actually abort t6044: add a testcase for index matching head, when head doesn't match HEAD t6044: verify that merges expected to abort actually abort index_has_changes(): avoid assuming operating on the_index read-cache.c: move index_has_changes() from merge.c
2018-07-03read-cache.c: move index_has_changes() from merge.cLibravatar Elijah Newren1-31/+0
Since index_has_change() is an index-related function, move it to read-cache.c, only modifying it to avoid uses of the active_cache and active_nr macros. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-30Merge branch 'ma/unpack-trees-free-msgs'Libravatar Junio C Hamano1-15/+20
Leak plugging. * ma/unpack-trees-free-msgs: unpack_trees_options: free messages when done argv-array: return the pushed string from argv_push*() merge-recursive: provide pair of `unpack_trees_{start,finish}()` merge: setup `opts` later in `checkout_fast_forward()`
2018-05-22unpack_trees_options: free messages when doneLibravatar Martin Ågren1-0/+3
The strings allocated in `setup_unpack_trees_porcelain()` are never freed. Provide a function `clear_unpack_trees_porcelain()` to do so and call it where we use `setup_unpack_trees_porcelain()`. The only non-trivial user is `unpack_trees_start()`, where we should place the new call in `unpack_trees_finish()`. We keep the string pointers in an array, mixing pointers to static memory and memory that we allocate on the heap. We also keep several copies of the individual pointers. So we need to make sure that we do not free what we must not free and that we do not double-free. Let a separate argv_array take ownership of all the strings we create so that we can easily free them. Zero the whole array of string pointers to make sure that we do not leave any dangling pointers. Note that we only take responsibility for the memory allocated in `setup_unpack_trees_porcelain()` and not any other members of the `struct unpack_trees_options`. Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-21merge: setup `opts` later in `checkout_fast_forward()`Libravatar Martin Ågren1-15/+17
After we initialize the various fields in `opts` but before we actually use them, we might return early. Move the initialization further down, to immediately before we use `opts`. This limits the scope of `opts` and will help a later commit fix a memory leak without having to worry about those early returns. This patch is best viewed using something like this (note the tab!): --color-moved --anchored=" trees[nr_trees] = parse_tree_indirect" Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-02merge: convert empty tree constant to the_hash_algoLibravatar brian m. carlson1-4/+1
To avoid dependency on a particular hash algorithm, convert a use of EMPTY_TREE_SHA1_HEX to use the_hash_algo->empty_tree instead. Since both branches now use oid_to_hex, condense the if statement into a ternary. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-02-28merge: always roll back lock in `checkout_fast_forward()`Libravatar Martin Ågren1-3/+9
This function originated in builtin/merge.c. It was moved to merge.c in commit db699a8a1f (Move try_merge_command and checkout_fast_forward to libgit.a, 2012-10-26), but was used from sequencer.c even before that. If a problem occurs, the function returns without rolling back the lockfile. Teach it to do so. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-12-22Merge branch 'ew/empty-merge-with-dirty-index-maint' into ↵Libravatar Junio C Hamano1-0/+33
ew/empty-merge-with-dirty-index * ew/empty-merge-with-dirty-index-maint: merge-recursive: avoid incorporating uncommitted changes in a merge move index_has_changes() from builtin/am.c to merge.c for reuse t6044: recursive can silently incorporate dirty changes in a merge
2017-12-22move index_has_changes() from builtin/am.c to merge.c for reuseLibravatar Elijah Newren1-0/+33
index_has_changes() is a function we want to reuse outside of just am, making it also available for merge-recursive and merge-ort. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-10-07read-cache: leave lock in right state in `write_locked_index()`Libravatar Martin Ågren1-3/+1
If the original version of `write_locked_index()` returned with an error, it didn't roll back the lockfile unless the error occured at the very end, during closing/committing. See commit 03b866477 (read-cache: new API write_locked_index instead of write_index/write_cache, 2014-06-13). In commit 9f41c7a6b (read-cache: close index.lock in do_write_index, 2017-04-26), we learned to close the lock slightly earlier in the callstack. That was mostly a side-effect of lockfiles being implemented using temporary files, but didn't cause any real harm. Recently, commit 076aa2cbd (tempfile: auto-allocate tempfiles on heap, 2017-09-05) introduced a subtle bug. If the temporary file is deleted (i.e., the lockfile is rolled back), the tempfile-pointer in the `struct lock_file` will be left dangling. Thus, an attempt to reuse the lockfile, or even just to roll it back, will induce undefined behavior -- most likely a crash. Besides not crashing, we clearly want to make things consistent. The guarantees which the lockfile-machinery itself provides is A) if we ask to commit and it fails, roll back, and B) if we ask to close and it fails, do _not_ roll back. Let's do the same for consistency. Do not delete the temporary file in `do_write_index()`. One of its callers, `write_locked_index()` will thereby avoid rolling back the lock. The other caller, `write_shared_index()`, will delete its temporary file anyway. Both of these callers will avoid undefined behavior (crashing). Teach `write_locked_index(..., COMMIT_LOCK)` to roll back the lock before returning. If we have already succeeded and committed, it will be a noop. Simplify the existing callers where we now have a superfluous call to `rollback_lockfile()`. That should keep future readers from wondering why the callers are inconsistent. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-10-06treewide: prefer lockfiles on the stackLibravatar Martin Ågren1-4/+4
There is no longer any need to allocate and leak a `struct lock_file`. The previous patch addressed an instance where we needed a minor tweak alongside the trivial changes. Deal with the remaining instances where we allocate and leak a struct within a single function. Change them to have the `struct lock_file` on the stack instead. These instances were identified by running `git grep "^\s*struct lock_file\s*\*"`. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-05-08tree: convert parse_tree_indirect to struct object_idLibravatar brian m. carlson1-2/+2
Convert parse_tree_indirect to take a pointer to struct object_id. Update all the callers. This transformation was achieved using the following semantic patch and manual updates to the declaration and definition. Update builtin/checkout.c manually as well, since it uses a ternary expression not handled by the semantic patch. @@ expression E1; @@ - parse_tree_indirect(E1.hash) + parse_tree_indirect(&E1) @@ expression E1; @@ - parse_tree_indirect(E1->hash) + parse_tree_indirect(E1) Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-05-08merge: convert checkout_fast_forward to struct object_idLibravatar brian m. carlson1-4/+4
Converting checkout_fast_forward is required to convert parse_tree_indirect. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-12-07lockfile: LOCK_REPORT_ON_ERRORLibravatar Junio C Hamano1-1/+1
The "libify sequencer" topic stopped passing the die_on_error option to hold_locked_index(), and this lost an error message from "git merge --ff-only $commit" when there are competing updates in progress. The command still exits with a non-zero status, but that is not of much help for an interactive user. The last thing the command says is "Updating $from..$to". We used to follow it with a big error message that makes it clear that "merge --ff-only" did not succeed. What is sad is that we should have noticed this regression while reviewing the change. It was clear that the update to the checkout_fast_forward() function made a failing hold_locked_index() silent, but the only caller of the checkout_fast_forward() function had this comment: if (checkout_fast_forward(from, to, 1)) - exit(128); /* the callee should have complained already */ + return -1; /* the callee should have complained already */ which clearly contradicted the assumption X-<. Add a new option LOCK_REPORT_ON_ERROR that can be passed instead of LOCK_DIE_ON_ERROR to the hold_lock*() family of functions and teach checkout_fast_forward() to use it to fix this regression. After going thourgh all calls to hold_lock*() family of functions that used to pass LOCK_DIE_ON_ERROR but were modified to pass 0 in the "libify sequencer" topic "git show --first-parent 2a4062a4a8", it appears that this is the only one that has become silent. Many others used to give detailed report that talked about "there may be competing Git process running" but with the series merged they now only give a single liner "Unable to lock ...", some of which may have to be tweaked further, but at least they say something, unlike the one this patch fixes. Reported-by: Robbie Iannucci <iannucci@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-09sequencer: lib'ify checkout_fast_forward()Libravatar Johannes Schindelin1-3/+6
Instead of dying there, let the caller high up in the callchain notice the error and handle it (by dying, still). The only callers of checkout_fast_forward(), cmd_merge(), pull_into_void(), cmd_pull() and sequencer's fast_forward_to(), already check the return value and handle it appropriately. With this step, we make it notice an error return from this function. So this is a safe conversion to make checkout_fast_forward() callable from new callers that want it not to die, without changing the external behaviour of anything existing. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-11-20Convert struct object to object_idLibravatar brian m. carlson1-1/+1
struct object is one of the major data structures dealing with object IDs. Convert it to use struct object_id instead of an unsigned char array. Convert get_object_hash to refer to the new member as well. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Jeff King <peff@peff.net>
2014-10-01lockfile.h: extract new header file for the functions in lockfile.cLibravatar Michael Haggerty1-0/+1
Move the interface declaration for the functions in lockfile.c from cache.h to a new file, lockfile.h. Add #includes where necessary (and remove some redundant includes of cache.h by files that already include builtin.h). Move the documentation of the lock_file state diagram from lockfile.c to the new header file. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-07-16Merge branch 'nd/split-index'Libravatar Junio C Hamano1-4/+3
An experiment to use two files (the base file and incremental changes relative to it) to represent the index to reduce I/O cost of rewriting a large index when only small part of the working tree changes. * nd/split-index: (32 commits) t1700: new tests for split-index mode t2104: make sure split index mode is off for the version test read-cache: force split index mode with GIT_TEST_SPLIT_INDEX read-tree: note about dropping split-index mode or index version read-tree: force split-index mode off on --index-output rev-parse: add --shared-index-path to get shared index path update-index --split-index: do not split if $GIT_DIR is read only update-index: new options to enable/disable split index mode split-index: strip pathname of on-disk replaced entries split-index: do not invalidate cache-tree at read time split-index: the reading part split-index: the writing part read-cache: mark updated entries for split index read-cache: save deleted entries in split index read-cache: mark new entries for split index read-cache: split-index mode read-cache: save index SHA-1 after reading entry.c: update cache_changed if refresh_cache is set in checkout_entry() cache-tree: mark istate->cache_changed on prime_cache_tree() cache-tree: mark istate->cache_changed on cache tree update ...
2014-06-19merge: use argv_array when spawning merge strategyLibravatar Jeff King1-29/+13
This is shorter, and avoids a rather complicated set of allocation and free steps. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-06-13read-cache: new API write_locked_index instead of write_index/write_cacheLibravatar Nguyễn Thái Ngọc Duy1-4/+3
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-10-29Move try_merge_command and checkout_fast_forward to libgit.aLibravatar Nguyễn Thái Ngọc Duy1-0/+112
These functions are called in sequencer.c, which is part of libgit.a. This makes libgit.a potentially require builtin/merge.c for external git commands. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Jeff King <peff@peff.net>