summaryrefslogtreecommitdiff
path: root/builtin/name-rev.c
AgeCommit message (Collapse)AuthorFilesLines
2020-02-05name-rev: sort tip names before applyingLibravatar René Scharfe1-8/+52
name_ref() is called for each ref and checks if its a better name for the referenced commit. If that's the case it remembers it and checks if a name based on it is better for its ancestors as well. This in done in the the order for_each_ref() imposes on us. That might not be optimal. If bad names happen to be encountered first (as defined by is_better_name()), names derived from them may spread to a lot of commits, only to be replaced by better names later. Setting better names first can avoid that. is_better_name() prefers tags, short distances and old references. The distance is a measure that we need to calculate for each candidate commit, but the other two properties are not dependent on the relationships of commits. Sorting the refs by them should yield better performance than the essentially random order we currently use. And applying older references first should also help to reduce rework due to the fact that older commits have less ancestors than newer ones. So add all details of names to the tip table first, then sort them to prefer tags and older references and then apply them in this order. Here's the performance as measures by hyperfine for the Linux repo before: Benchmark #1: ./git -C ../linux/ name-rev --all Time (mean ± σ): 851.1 ms ± 4.5 ms [User: 806.7 ms, System: 44.4 ms] Range (min … max): 845.9 ms … 859.5 ms 10 runs ... and with this patch: Benchmark #1: ./git -C ../linux/ name-rev --all Time (mean ± σ): 736.2 ms ± 8.7 ms [User: 688.4 ms, System: 47.5 ms] Range (min … max): 726.0 ms … 755.2 ms 10 runs Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-05name-rev: release unused name stringsLibravatar René Scharfe1-5/+16
name_rev() assigns a name to a commit and its parents and grandparents and so on. Commits share their name string with their first parent, which in turn does the same, recursively to the root. That saves a lot of allocations. When a better name is found, the old name is replaced, but its memory is not released. That leakage can become significant. Can we release these old strings exactly once even though they are referenced multiple times? Yes, indeed -- we can make use of the fact that name_rev() visits the ancestors of a commit after it set a new name for it and tries to update their names as well. Members of the first ancestral line have the same taggerdate and from_tag values, but a higher distance value than their child commit at generation 0. These are the only criteria used by is_better_name(). Lower distance values are considered better, so a name that is better for a child will also be better for its parent and grandparent etc. That means we can free(3) an inferior name at generation 0 and rely on name_rev() to replace all references in ancestors as well. If we do that then we need to stop using the string pointer alone to distinguish new empty rev_name slots from initialized ones, though, as it technically becomes invalid after the free(3) call -- even though its value is still different from NULL. We can check the generation value first, as empty slots will have it initialized to 0, and for the actual generation 0 we'll set a new valid name right after the create_or_update_name() call that releases the string. For the Chromium repo, releasing superceded names reduces the memory footprint of name-rev --all significantly. Here's the output of GNU time before: 0.98user 0.48system 0:01.46elapsed 99%CPU (0avgtext+0avgdata 2601812maxresident)k 0inputs+0outputs (0major+571470minor)pagefaults 0swaps ... and with this patch: 1.01user 0.26system 0:01.28elapsed 100%CPU (0avgtext+0avgdata 1559196maxresident)k 0inputs+0outputs (0major+314370minor)pagefaults 0swaps It also gets faster; hyperfine before: Benchmark #1: ./git -C ../chromium/src name-rev --all Time (mean ± σ): 1.534 s ± 0.006 s [User: 1.039 s, System: 0.494 s] Range (min … max): 1.522 s … 1.542 s 10 runs ... and with this patch: Benchmark #1: ./git -C ../chromium/src name-rev --all Time (mean ± σ): 1.338 s ± 0.006 s [User: 1.047 s, System: 0.291 s] Range (min … max): 1.327 s … 1.346 s 10 runs For the Linux repo it doesn't pay off; memory usage only gets down from: 0.76user 0.03system 0:00.80elapsed 99%CPU (0avgtext+0avgdata 292848maxresident)k 0inputs+0outputs (0major+44579minor)pagefaults 0swaps ... to: 0.78user 0.03system 0:00.81elapsed 100%CPU (0avgtext+0avgdata 284696maxresident)k 0inputs+0outputs (0major+44892minor)pagefaults 0swaps The runtime actually increases slightly from: Benchmark #1: ./git -C ../linux/ name-rev --all Time (mean ± σ): 828.8 ms ± 5.0 ms [User: 797.2 ms, System: 31.6 ms] Range (min … max): 824.1 ms … 838.9 ms 10 runs ... to: Benchmark #1: ./git -C ../linux/ name-rev --all Time (mean ± σ): 847.6 ms ± 3.4 ms [User: 807.9 ms, System: 39.6 ms] Range (min … max): 843.4 ms … 854.3 ms 10 runs Why is that? In the Chromium repo, ca. 44000 free(3) calls in create_or_update_name() release almost 1GB, while in the Linux repo 240000+ calls release a bit more than 5MB, so the average discarded name is ca. 1000x longer in the latter. Overall I think it's the right tradeoff to make, as it helps curb the memory usage in repositories with big discarded names, and the added overhead is small. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-05name-rev: generate name strings only if they are betterLibravatar René Scharfe1-17/+18
Leave setting the tip_name member of struct rev_name to callers of create_or_update_name(). This avoids allocations for names that are rejected by that function. Here's how this affects the runtime when working with a fresh clone of Git's own repository; performance numbers by hyperfine before: Benchmark #1: ./git -C ../git-pristine/ name-rev --all Time (mean ± σ): 437.8 ms ± 4.0 ms [User: 422.5 ms, System: 15.2 ms] Range (min … max): 432.8 ms … 446.3 ms 10 runs ... and with this patch: Benchmark #1: ./git -C ../git-pristine/ name-rev --all Time (mean ± σ): 408.5 ms ± 1.4 ms [User: 387.2 ms, System: 21.2 ms] Range (min … max): 407.1 ms … 411.7 ms 10 runs Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-05name-rev: pre-size buffer in get_parent_name()Libravatar René Scharfe1-6/+14
We can calculate the size of new name easily and precisely. Open-code the xstrfmt() calls and grow the buffers as needed before filling them. This provides a surprisingly large benefit when working with the Chromium repository; here are the numbers measured using hyperfine before: Benchmark #1: ./git -C ../chromium/src name-rev --all Time (mean ± σ): 5.822 s ± 0.013 s [User: 5.304 s, System: 0.516 s] Range (min … max): 5.803 s … 5.837 s 10 runs ... and with this patch: Benchmark #1: ./git -C ../chromium/src name-rev --all Time (mean ± σ): 1.527 s ± 0.003 s [User: 1.015 s, System: 0.511 s] Range (min … max): 1.524 s … 1.535 s 10 runs Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-05name-rev: factor out get_parent_name()Libravatar René Scharfe1-13/+14
Reduce nesting by moving code to come up with a name for the parent into its own function. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-05name-rev: put struct rev_name into commit slabLibravatar René Scharfe1-9/+11
The commit slab commit_rev_name contains a pointer to a struct rev_name, and the actual struct is allocated separatly. Avoid that allocation and pointer indirection by storing the full struct in the commit slab. Use the tip_name member pointer to determine if the returned struct is initialized. Performance in the Linux repository measured with hyperfine before: Benchmark #1: ./git -C ../linux/ name-rev --all Time (mean ± σ): 953.5 ms ± 6.3 ms [User: 901.2 ms, System: 52.1 ms] Range (min … max): 945.2 ms … 968.5 ms 10 runs ... and with this patch: Benchmark #1: ./git -C ../linux/ name-rev --all Time (mean ± σ): 851.0 ms ± 3.1 ms [User: 807.4 ms, System: 43.6 ms] Range (min … max): 846.7 ms … 857.0 ms 10 runs Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-05name-rev: don't _peek() in create_or_update_name()Libravatar René Scharfe1-10/+4
Look up the commit slab slot for the commit once using commit_rev_name_at() and populate it in case it is empty, instead of checking for emptiness in a separate step using commit_rev_name_peek() via get_commit_rev_name(). Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-05name-rev: don't leak path copy in name_ref()Libravatar René Scharfe1-1/+3
name_ref() duplicates the path string and passes it to name_rev(), which either puts it into a commit slab or ignores it if there is already a better name, leaking it. Move the duplication to name_rev() and release the copy in the latter case. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-05name-rev: respect const qualifierLibravatar René Scharfe1-3/+3
Keep the const qualifier of the first parameter of get_rev_name() even when casting the object pointer to a commit pointer, and further for the parameter of get_commit_rev_name(), as all these uses are read-only. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-05name-rev: remove unused typedefLibravatar René Scharfe1-2/+2
The type alias became unused with bf43abc6e6 (name-rev: use sizeof(*ptr) instead of sizeof(type) in allocation, 2019-11-12); remove it. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-05name-rev: rewrite create_or_update_name()Libravatar Martin Ågren1-12/+12
This code was moved straight out of name_rev(). As such, we inherited the "goto" to jump from an if into an else-if. We also inherited the fact that "nothing to do -- return NULL" is handled last. Rewrite the function to first handle the "nothing to do" case. Then we can handle the conditional allocation early before going on to populate the struct. No need for goto-ing. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-25Merge branch 'sg/name-rev-wo-recursion'Libravatar Junio C Hamano1-50/+97
Redo "git name-rev" to avoid recursive calls. * sg/name-rev-wo-recursion: name-rev: cleanup name_ref() name-rev: eliminate recursion in name_rev() name-rev: use 'name->tip_name' instead of 'tip_name' name-rev: drop name_rev()'s 'generation' and 'distance' parameters name-rev: restructure creating/updating 'struct rev_name' instances name-rev: restructure parsing commits and applying date cutoff name-rev: pull out deref handling from the recursion name-rev: extract creating/updating a 'struct name_rev' into a helper t6120: add a test to cover inner conditions in 'git name-rev's name_rev() name-rev: use sizeof(*ptr) instead of sizeof(type) in allocation name-rev: avoid unnecessary cast in name_ref() name-rev: use strbuf_strip_suffix() in get_rev_name() t6120-describe: modernize the 'check_describe' helper t6120-describe: correct test repo history graph in comment
2019-12-09name-rev: cleanup name_ref()Libravatar SZEDER Gábor1-15/+16
Earlier patches in this series moved a couple of conditions from the recursive name_rev() function into its caller name_ref(), for no other reason than to make eliminating the recursion a bit easier to follow. Since the previous patch name_rev() is not recursive anymore, so let's move all those conditions back into name_rev(). Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-09name-rev: eliminate recursion in name_rev()Libravatar SZEDER Gábor1-38/+64
The name_rev() function calls itself recursively for each interesting parent of the commit it got as parameter, and, consequently, it can segfault when processing a deep history if it exhausts the available stack space. E.g. running 'git name-rev --all' and 'git name-rev HEAD~100000' in the gcc, gecko-dev, llvm, and WebKit repositories results in segfaults on my machine ('ulimit -s' reports 8192kB of stack size limit), and nowadays the former segfaults in the Linux repo as well (it reached the necessasry depth sometime between v5.3-rc4 and -rc5). Eliminate the recursion by inserting the interesting parents into a LIFO 'prio_queue' [1] and iterating until the queue becomes empty. Note that the parent commits must be added in reverse order to the LIFO 'prio_queue', so their relative order is preserved during processing, i.e. the first parent should come out first from the queue, because otherwise performance greatly suffers on mergy histories [2]. The stacksize-limited test 'name-rev works in a deep repo' in 't6120-describe.sh' demonstrated this issue and expected failure. Now the recursion is gone, so flip it to expect success. Also gone are the dmesg entries logging the segfault of that segfaulting 'git name-rev' process on every execution of the test suite. Note that this slightly changes the order of lines in the output of 'git name-rev --all', usually swapping two lines every 35 lines in git.git or every 150 lines in linux.git. This shouldn't matter in practice, because the output has always been unordered anyway. This patch is best viewed with '--ignore-all-space'. [1] Early versions of this patch used a 'commit_list', resulting in ~15% performance penalty for 'git name-rev --all' in 'linux.git', presumably because of the memory allocation and release for each insertion and removal. Using a LIFO 'prio_queue' has basically no effect on performance. [2] We prefer shorter names, i.e. 'v0.1~234' is preferred over 'v0.1^2~5', meaning that usually following the first parent of a merge results in the best name for its ancestors. So when later we follow the remaining parent(s) of a merge, and reach an already named commit, then we usually find that we can't give that commit a better name, and thus we don't have to visit any of its ancestors again. OTOH, if we were to follow the Nth parent of the merge first, then the name of all its ancestors would include a corresponding '^N'. Those are not the best names for those commits, so when later we reach an already named commit following the first parent of that merge, then we would have to update the name of that commit and the names of all of its ancestors as well. Consequently, we would have to visit many commits several times, resulting in a significant slowdown. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-09name-rev: use 'name->tip_name' instead of 'tip_name'Libravatar SZEDER Gábor1-4/+7
Following the previous patches in this series we can get the value of 'name_rev()'s 'tip_name' parameter from the 'struct rev_name' associated with the commit as well. So let's use 'name->tip_name' instead, which makes the patch eliminating the recursion of name_rev() a bit easier to follow. Note that at this point we could drop the 'tip_name' parameter as well, but that parameter will be necessary later, after the recursion is eliminated. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06name-rev: drop name_rev()'s 'generation' and 'distance' parametersLibravatar SZEDER Gábor1-12/+13
Following the previous patches in this series we can get the values of name_rev()'s 'generation' and 'distance' parameters from the 'stuct rev_name' associated with the commit as well. Let's simplify the function's signature and remove these two unnecessary parameters. Note that at this point we could do the same with the 'tip_name', 'taggerdate' and 'from_tag' parameters as well, but those parameters will be necessary later, after the recursion is eliminated. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06name-rev: restructure creating/updating 'struct rev_name' instancesLibravatar SZEDER Gábor1-14/+21
At the beginning of the recursive name_rev() function it creates a new 'struct rev_name' instance for each previously unvisited commit or, if this visit results in better name for an already visited commit, then updates the 'struct rev_name' instance attached to the commit, or returns early. Restructure this so it's caller creates or updates the 'struct rev_name' instance associated with the commit to be passed as parameter, i.e. both name_ref() before calling name_rev() and name_rev() itself as it iterates over the parent commits. This makes eliminating the recursion a bit easier to follow, and the condition moved to name_ref() will be moved back to name_rev() after the recursion is eliminated. This change also plugs the memory leak that was temporarily unplugged in the earlier "name-rev: pull out deref handling from the recursion" patch in this series. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06name-rev: restructure parsing commits and applying date cutoffLibravatar SZEDER Gábor1-13/+16
At the beginning of the recursive name_rev() function it parses the commit it got as parameter, and returns early if the commit is older than a cutoff limit. Restructure this so the caller parses the commit and checks its date, and doesn't invoke name_rev() if the commit to be passed as parameter is older than the cutoff, i.e. both name_ref() before calling name_rev() and name_rev() itself as it iterates over the parent commits. This makes eliminating the recursion a bit easier to follow, and the condition moved to name_ref() will be moved back to name_rev() after the recursion is eliminated. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06name-rev: pull out deref handling from the recursionLibravatar SZEDER Gábor1-17/+10
The 'if (deref) { ... }' condition near the beginning of the recursive name_rev() function can only ever be true in the first invocation, because the 'deref' parameter is always 0 in the subsequent recursive invocations. Extract this condition from the recursion into name_rev()'s caller and drop the function's 'deref' parameter. This makes eliminating the recursion a bit easier to follow, and it will be moved back into name_rev() after the recursion is eliminated. Furthermore, drop the condition that die()s when both 'deref' and 'generation' are non-null (which should have been a BUG() to begin with). Note that this change reintroduces the memory leak that was plugged in in commit 5308224633 (name-rev: avoid leaking memory in the `deref` case, 2017-05-04), but a later patch (name-rev: restructure creating/updating 'struct rev_name' instances) in this series will plug it in again. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06name-rev: extract creating/updating a 'struct name_rev' into a helperLibravatar SZEDER Gábor1-13/+27
In a later patch in this series we'll want to do this in two places. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06name-rev: use sizeof(*ptr) instead of sizeof(type) in allocationLibravatar SZEDER Gábor1-1/+1
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06name-rev: avoid unnecessary cast in name_ref()Libravatar SZEDER Gábor1-1/+1
Casting a 'struct object' to 'struct commit' is unnecessary there, because it's already available in the local 'commit' variable. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06name-rev: use strbuf_strip_suffix() in get_rev_name()Libravatar René Scharfe1-4/+3
get_name_rev() basically open-codes strip_suffix() before adding a string to a strbuf. Let's use the strbuf right from the beginning, i.e. add the whole string to the strbuf and then use strbuf_strip_suffix(), making the code more idiomatic. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-27name-rev: use skip_prefix() instead of starts_with()Libravatar René Scharfe1-4/+4
Let skip_prefix() advance refname to get rid of two magic numbers. Signed-off-by: René Scharfe <l.s.r@web.de> Acked-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-09-28name-rev: avoid cutoff timestamp underflowLibravatar SZEDER Gábor1-3/+12
When 'git name-rev' is invoked with commit-ish parameters, it tries to save some work, and doesn't visit commits older than the committer date of the oldest given commit minus a one day worth of slop. Since our 'timestamp_t' is an unsigned type, this leads to a timestamp underflow when the committer date of the oldest given commit is within a day of the UNIX epoch. As a result the cutoff timestamp ends up far-far in the future, and 'git name-rev' doesn't visit any commits, and names each given commit as 'undefined'. Check whether subtracting the slop from the oldest committer date would lead to an underflow, and use no cutoff in that case. We don't have a TIME_MIN constant, dddbad728c (timestamp_t: a new data type for timestamps, 2017-04-26) didn't add one, so do it now. Note that the type of the cutoff timestamp variable used to be signed before 5589e87fd8 (name-rev: change a "long" variable to timestamp_t, 2017-05-20). The behavior was still the same even back then, but the underflow didn't happen when substracting the slop from the oldest committer date, but when comparing the signed cutoff timestamp with unsigned committer dates in name_rev(). IOW, this underflow bug is as old as 'git name-rev' itself. Helped-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-06-20object: convert lookup_object() to use object_idLibravatar Jeff King1-2/+1
There are no callers left of lookup_object() that aren't just passing us the "hash" member of a "struct object_id". Let's take the whole struct, which gets us closer to removing all raw sha1 variables. It also matches the existing conversions of lookup_blob(), etc. The conversions of callers were done by hand, but they're all mechanical one-liners. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-05-13name-rev: drop unused parameters from is_better_name()Libravatar Jeff King1-4/+1
When this function was extracted in 0041bf6544 (name-rev: refactor logic to see if a new candidate is a better name, 2017-03-29), it ended up getting more arguments than it needs. It's possible we may later use these values to evaluate the name, but since it's a static function with a single caller, it will be easy to add them back then. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-04-01builtin/name-rev: make hash-size independentLibravatar brian m. carlson1-6/+8
Use the_hash_algo when parsing instead of GIT_SHA1_HEXSZ so that this function works with any size hash. Rename the variable forty to counter, as this is a better name and is independent of the hash size. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-06-29tag: add repository argument to deref_tagLibravatar Stefan Beller1-1/+2
Add a repository argument to allow the callers of deref_tag to be more specific about which repository to act on. This is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet. As with the previous commits, use a macro to catch callers passing a repository other than the_repository at compile time. Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-06-29object: add repository argument to lookup_objectLibravatar Stefan Beller1-1/+2
Add a repository argument to allow callers of lookup_object to be more specific about which repository to handle. This is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet. As with the previous commits, use a macro to catch callers passing a repository other than the_repository at compile time. Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-06-29object: add repository argument to parse_objectLibravatar Stefan Beller1-3/+4
Add a repository argument to allow the callers of parse_object to be more specific about which repository to act on. This is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet. As with the previous commits, use a macro to catch callers passing a repository other than the_repository at compile time. Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-21name-rev: use commit-slab for rev-name instead of commit->utilLibravatar Nguyễn Thái Ngọc Duy1-3/+20
It's done so that commit->util can be removed. See more explanation in the commit that removes commit->util. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-03-14Convert find_unique_abbrev* to struct object_idLibravatar brian m. carlson1-1/+1
Convert find_unique_abbrev and find_unique_abbrev_r to each take a pointer to struct object_id. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-10-07Merge branch 'ma/builtin-unleak'Libravatar Junio C Hamano1-0/+1
Many variables that points at a region of memory that will live throughout the life of the program have been marked with UNLEAK marker to help the leak checkers concentrate on real leaks.. * ma/builtin-unleak: builtin/: add UNLEAKs
2017-10-02builtin/: add UNLEAKsLibravatar Martin Ågren1-0/+1
Add some UNLEAKs where we are about to return from `cmd_*`. UNLEAK the variables in the same order as we've declared them. While addressing `msg` in builtin/tag.c, convert the existing `strbuf_release()` calls as well. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-09-19Merge branch 'mg/timestamp-t-fix'Libravatar Junio C Hamano1-1/+1
A mismerge fix. * mg/timestamp-t-fix: name-rev: change ULONG_MAX to TIME_MAX
2017-09-06name-rev: change ULONG_MAX to TIME_MAXLibravatar Michael J Gruber1-1/+1
Earlier, dddbad728c ("timestamp_t: a new data type for timestamps", 2017-04-26) changed several types to timestamp_t. 5589e87fd8 ("name-rev: change a "long" variable to timestamp_t", 2017-05-20) cleaned up a missed variable, but both missed a _MAX constant. Change the remaining constant to the one appropriate for the current type Signed-off-by: Michael J Gruber <git@grubix.eu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-07-10Merge branch 'ab/wildmatch'Libravatar Junio C Hamano1-1/+1
Minor code cleanup. * ab/wildmatch: wildmatch: remove unused wildopts parameter
2017-06-24Merge branch 'bw/config-h'Libravatar Junio C Hamano1-0/+1
Fix configuration codepath to pay proper attention to commondir that is used in multi-worktree situation, and isolate config API into its own header file. * bw/config-h: config: don't implicitly use gitdir or commondir config: respect commondir setup: teach discover_git_directory to respect the commondir config: don't include config.h by default config: remove git_config_iter config: create config.h
2017-06-23wildmatch: remove unused wildopts parameterLibravatar Ævar Arnfjörð Bjarmason1-1/+1
Remove the unused wildopts placeholder struct from being passed to all wildmatch() invocations, or rather remove all the boilerplate NULL parameters. This parameter was added back in commit 9b3497cab9 ("wildmatch: rename constants and update prototype", 2013-01-01) as a placeholder for future use. Over 4 years later nothing has made use of it, let's just remove it. It can be added in the future if we find some reason to start using such a parameter. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-15config: don't include config.h by defaultLibravatar Brandon Williams1-0/+1
Stop including config.h by default in cache.h. Instead only include config.h in those files which require use of the config system. Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-05-30Merge branch 'js/larger-timestamps'Libravatar Junio C Hamano1-1/+1
A follow-up hotfix for a topic already in 'master'. * js/larger-timestamps: name-rev: change a "long" variable to timestamp_t
2017-05-30Merge branch 'jc/name-rev-lw-tag'Libravatar Junio C Hamano1-7/+52
"git describe --contains" penalized light-weight tags so much that they were almost never considered. Instead, give them about the same chance to be considered as an annotated tag that is the same age as the underlying commit would. * jc/name-rev-lw-tag: name-rev: favor describing with tags and use committer date to tiebreak name-rev: refactor logic to see if a new candidate is a better name
2017-05-29Merge branch 'js/plug-leaks'Libravatar Junio C Hamano1-2/+5
Fix memory leaks pointed out by Coverity (and people). * js/plug-leaks: (26 commits) checkout: fix memory leak submodule_uses_worktrees(): plug memory leak show_worktree(): plug memory leak name-rev: avoid leaking memory in the `deref` case remote: plug memory leak in match_explicit() add_reflog_for_walk: avoid memory leak shallow: avoid memory leak line-log: avoid memory leak receive-pack: plug memory leak in update() fast-export: avoid leaking memory in handle_tag() mktree: plug memory leaks reported by Coverity pack-redundant: plug memory leak setup_discovered_git_dir(): plug memory leak setup_bare_git_dir(): help static analysis split_commit_in_progress(): simplify & fix memory leak checkout: fix memory leak cat-file: fix memory leak mailinfo & mailsplit: check for EOF while parsing status: close file descriptor after reading git-rebase-todo difftool: address a couple of resource/memory leaks ...
2017-05-29Merge branch 'bc/object-id'Libravatar Junio C Hamano1-16/+16
Conversion from uchar[20] to struct object_id continues. * bc/object-id: (53 commits) object: convert parse_object* to take struct object_id tree: convert parse_tree_indirect to struct object_id sequencer: convert do_recursive_merge to struct object_id diff-lib: convert do_diff_cache to struct object_id builtin/ls-tree: convert to struct object_id merge: convert checkout_fast_forward to struct object_id sequencer: convert fast_forward_to to struct object_id builtin/ls-files: convert overlay_tree_on_cache to object_id builtin/read-tree: convert to struct object_id sha1_name: convert internals of peel_onion to object_id upload-pack: convert remaining parse_object callers to object_id revision: convert remaining parse_object callers to object_id revision: rename add_pending_sha1 to add_pending_oid http-push: convert process_ls_object and descendants to object_id refs/files-backend: convert many internals to struct object_id refs: convert struct ref_update to use struct object_id ref-filter: convert some static functions to struct object_id Convert struct ref_array_item to struct object_id Convert the verify_pack callback to struct object_id Convert lookup_tag to struct object_id ...
2017-05-20name-rev: change a "long" variable to timestamp_tLibravatar Junio C Hamano1-1/+1
Earlier dddbad72 ("timestamp_t: a new data type for timestamps", 2017-04-26) updated all in-core variables, fields and function return values that are used to store "seconds since epoch" to a new type timestamp_t. Unfortunately one variable "cutoff", which is used to keep track of the oldest timestamp of commit we saw on the command line, was "long" and left behind. Update it to timestamp_t as well. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-05-08object: convert parse_object* to take struct object_idLibravatar brian m. carlson1-3/+3
Make parse_object, parse_object_or_die, and parse_object_buffer take a pointer to struct object_id. Remove the temporary variables inserted earlier, since they are no longer necessary. Transform all of the callers using the following semantic patch: @@ expression E1; @@ - parse_object(E1.hash) + parse_object(&E1) @@ expression E1; @@ - parse_object(E1->hash) + parse_object(E1) @@ expression E1, E2; @@ - parse_object_or_die(E1.hash, E2) + parse_object_or_die(&E1, E2) @@ expression E1, E2; @@ - parse_object_or_die(E1->hash, E2) + parse_object_or_die(E1, E2) @@ expression E1, E2, E3, E4, E5; @@ - parse_object_buffer(E1.hash, E2, E3, E4, E5) + parse_object_buffer(&E1, E2, E3, E4, E5) @@ expression E1, E2, E3, E4, E5; @@ - parse_object_buffer(E1->hash, E2, E3, E4, E5) + parse_object_buffer(E1, E2, E3, E4, E5) Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-05-08name-rev: avoid leaking memory in the `deref` caseLibravatar Johannes Schindelin1-2/+5
When the `name_rev()` function is asked to dereference the tip name, it allocates memory. But when it turns out that another tip already described the commit better than the current one, we forgot to release the memory. Pointed out by Coverity. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-05-02builtin/name-rev: convert to struct object_idLibravatar brian m. carlson1-14/+14
Convert all the uses of unsigned char [20] to struct object_id. Also, convert some hard-coded integers into constants. name_rev_line accepts a wide variety of free-form input and only interprets 40-character hex values, passing through everything else. Consequently, it is not a good candidate for parse_oid_hex, which is much stricter. This change is a prerequisite for converting parse_object. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-04-27timestamp_t: a new data type for timestampsLibravatar Johannes Schindelin1-3/+3
Git's source code assumes that unsigned long is at least as precise as time_t. Which is incorrect, and causes a lot of problems, in particular where unsigned long is only 32-bit (notably on Windows, even in 64-bit versions). So let's just use a more appropriate data type instead. In preparation for this, we introduce the new `timestamp_t` data type. By necessity, this is a very, very large patch, as it has to replace all timestamps' data type in one go. As we will use a data type that is not necessarily identical to `time_t`, we need to be very careful to use `time_t` whenever we interact with the system functions, and `timestamp_t` everywhere else. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>