What's cooking in git.git (Sep 2017, #01; Wed, 6)

Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. The ones marked with '.' do not appear in any of the integration branches, but I am still holding onto them. We are at week #5 of this cycle.

Re: [PATCH] config: use a static lock_file struct

Jeff King writes: > In the long run we may want to drop the "tempfiles must remain forever" > rule. This is certainly not the first time it has caused confusion or > leaks. And I don't think it's a fundamental issue, just the way the code > is written. But in the interim, this fix

Re: [PATCH] hashmap: add API to disable item counting when threaded

Jeff King writes: > On Sat, Sep 02, 2017 at 01:31:19AM +0200, Johannes Schindelin wrote: > >> BTW this made me think that we may have a problem in our code since >> switching from my original hashmap implementation to the bucket one added >> in 6a364ced497 (add a hashtable

Re: [PATCH 3/3] merge-recursive: change current file dir string_lists to hashmap

I needed this squashed into before I could even compile the resulting code. Perhaps my compiler is stale? merge-recursive.c:28:22: error: declaration does not declare anything [-Werror] struct hashmap_entry; ^ merge-recursive.c:29:7: error: flexible array member in

Re: [PATCH] name-rev: change ULONG_MAX to TIME_MAX

Michael J Gruber writes: > 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

Re: [PATCH] format-patch: use raw format for notes

Sam Bobroff writes: > If "--notes=..." is used with "git format-patch", the notes are > prefixed with the ref's local name and indented, which looks odd and > exposes the path of the ref. > > Extend the test that suppresses this behaviour so that it also catches > this

Re: [PATCH] parse-options: warn developers on negated options

Junio C Hamano writes: > Stefan Beller writes: > >> This patch disallows all no- options, but we could be more open and allow >> --no-options that have the NO_NEG bit set. > > "--no-foo" that does not take "--foo" is perhaps OK so should not > trigger

Re: [PATCHv2] builtin/merge: honor commit-msg hook for merges

Stefan Beller writes: > Junio writes: >> I didn't check how "merge --continue" is implemented, but we need to >> behave exactly the same way over there, too. Making sure that it is >> a case in t7504 may be a good idea, in addition to the test you >> added. > > After

Re: [PATCH] parse-options: warn developers on negated options

Stefan Beller writes: > This patch disallows all no- options, but we could be more open and allow > --no-options that have the NO_NEG bit set. "--no-foo" that does not take "--foo" is perhaps OK so should not trigger an error. A ("--no-foo", "--foo") pair is better

Re: git merge algorithm question

On Tue, Sep 5, 2017 at 5:53 PM, Daniel Biran wrote: > >>> I'm trying to better understand one of the merge algorithms as I had some >>> triumphs and tribulations with using a set of commands during a merge. >>> tldr: can a git merge -s recursive -X patience; // result in a

Re: [PATCH 0/10] towards clean leak-checker output

Jeff King writes: > In fact, the whole extract_argv0_path thing is pointless without > RUNTIME_PREFIX. So I think one reasonable fix is just: > > diff --git a/exec_cmd.c b/exec_cmd.c > index fb94aeba9c..09f05c3bc3 100644 > --- a/exec_cmd.c > +++ b/exec_cmd.c > @@ -5,7 +5,10 @@ >

Re: [PATCH] refs: make sure we never pass NULL to hashcpy

Thomas Gummerer writes: > gcc on arch linux (version 7.1.1) warns that a NULL argument is passed > as the second parameter of memcpy. > > In file included from refs.c:5:0: > refs.c: In function ‘ref_transaction_verify’: > cache.h:948:2: error: argument 2 null where non-null

Re: [PATCH] hashmap: add API to disable item counting when threaded

Jeff Hostetler writes: > From: Jeff Hostetler > > This is to address concerns raised by ThreadSanitizer on the > mailing list about threaded unprotected R/W access to map.size with my > previous > "disallow rehash" change

Re: [PATCHv2] pull: honor submodule.recurse config option

Nicolas Morey-Chaisemartin writes: > "git pull" supports a --recurse-submodules option but does not parse the > submodule.recurse configuration item to set the default for that option. > Meanwhile "git fetch" does support submodule.recurse, producing > confusing

git merge algorithm question

>> I'm trying to better understand one of the merge algorithms as I had some >> triumphs and tribulations with using a set of commands during a merge. tldr: >> can a git merge -s recursive -X patience; // result in a fast-forward merge? >> will --no-ff stop it >> >> So, the scenario is this:

Re: Empty directories in Git: Current guidance for historical commits?

Quoting Torsten Bögershausen : Is just dropping a ".gitignore" or "README" file in the empty directories during conversion still the most reasonable approach? A .gitignore will, but may cost 0.001% CPU time when running "git status". I have seen systems that create a file

[PATCHv2] builtin/merge: honor commit-msg hook for merges

Similar to 65969d43d1 (merge: honor prepare-commit-msg hook, 2011-02-14) merge should also honor the commit-msg hook: When a merge is stopped due to conflicts or --no-commit, the subsequent commit calls the commit-msg hook. However, it is not called after a clean merge. Fix this inconsistency by

[PATCH] parse-options: warn developers on negated options

When compiling with -DDEVELOPER set (enabled via the Makefile DEVELOPER switch), inspect options if they are negated and warn about them. 1. Negated options are usually are problem down the maintenance road: When a new negated option ("--no-foo") is introduced, then the option can be

Re: BUG: attempt to trim too many characters

On Tue, Sep 5, 2017 at 3:03 PM, Jeff King wrote: > > This probably fixes it: Yup. Thanks. Linus

Re: [PATCH 10/10] add UNLEAK annotation for reducing leak false positives

On Tue, Sep 5, 2017 at 6:05 AM, Jeff King wrote: > int main(void) nit of the day: s/void/int argc, char *argv/ or in case we do not want to emphasize the argument list s/void// as that adds no uninteresting things. > > In other words, you can do: > > int main(void) >

Re: BUG: attempt to trim too many characters

On Tue, Sep 05, 2017 at 02:55:08PM -0700, Linus Torvalds wrote: > On Tue, Sep 5, 2017 at 2:50 PM, Jeff King wrote: > > > > What version of git are you running? This should be fixed by 03df567fbf > > (for_each_bisect_ref(): don't trim refnames, 2017-06-18) which is in > > v2.14. >

Re: [Bug] commit-tree shouldn't append an extra newline to commit messages

Jeff King writes: > On Tue, Sep 05, 2017 at 12:57:21PM -0400, Ross Kabus wrote: > >> On Tue, Sep 5, 2017 at 11:36 AM, Jeff King wrote: >> >> > So I'd argue that "git commit -F" is doing a reasonable >> > thing, and "commit-tree -F" should probably change to match

Re: BUG: attempt to trim too many characters

On Tue, Sep 5, 2017 at 2:50 PM, Jeff King wrote: > > What version of git are you running? This should be fixed by 03df567fbf > (for_each_bisect_ref(): don't trim refnames, 2017-06-18) which is in > v2.14. I'm way more recent than 2.14. I'm at commit 238e487ea ("The fifth batch

Re: BUG: attempt to trim too many characters

On Tue, Sep 05, 2017 at 02:39:05PM -0700, Linus Torvalds wrote: > I just got this with > >gitk --bisect > > while doing some bisection on my current kernel. > > It happens with "git rev-parse --bisect" too, but interestingly, "git > log --bisect" works fine. > > I have not tried to figure

BUG: attempt to trim too many characters

I just got this with gitk --bisect while doing some bisection on my current kernel. It happens with "git rev-parse --bisect" too, but interestingly, "git log --bisect" works fine. I have not tried to figure out anything further, except that it was introduced by commit b9c8e7f2f

Re: [PATCH] builtin/merge: honor commit-msg hook for merges

Stefan Beller writes: > Similar to 65969d43d1 (merge: honor prepare-commit-msg hook, 2011-02-14) > merge should also honor the commit-msg hook; the reason is the same as > in that commit: When a merge is stopped due to conflicts or --no-commit, > the subsequent commit calls

Re: [PATCH v3 1/3] refs/files-backend: add longer-scoped copy of string to list

Jeff King writes: > I noticed the HEAD funniness, too, when looking at this earlier. I agree > with Junio that it's not quite consistent with the general rule of > "string list items point to their refnames", but I don't think it > matters in practice. Yup, we are on the same

Re: Git in Outreachy round 15?

Hi, On Sat, Sep 2, 2017 at 12:30 AM, Jeff King wrote: > > The big questions on whether we can make this happen are: > > 1. Do we have mentor interest? > > I'm willing to mentor, but I'd like to hear whether other people > are interested, as well. Either way I can take

[PATCH] builtin/merge: honor commit-msg hook for merges

Similar to 65969d43d1 (merge: honor prepare-commit-msg hook, 2011-02-14) merge should also honor the commit-msg hook; the reason is the same as in that commit: When a merge is stopped due to conflicts or --no-commit, the subsequent commit calls the commit-msg hook. However, it is not called after

Re: [Bug] commit-tree shouldn't append an extra newline to commit messages

Gmail mangled that patch of course... Ross Kabus Software Engineer www.aerotech.com | 412-968-2833 On Tue, Sep 5, 2017 at 4:57 PM, Ross Kabus wrote: > From: Ross Kabus > Date: Tue, 5 Sep 2017 13:54:52 -0400 > Subject: [PATCH] commit-tree: don't append

Re: [Bug] commit-tree shouldn't append an extra newline to commit messages

From: Ross Kabus Date: Tue, 5 Sep 2017 13:54:52 -0400 Subject: [PATCH] commit-tree: don't append a newline with -F This change makes it such that commit-tree -F never appends a newline character to the supplied commit message (either from file or stdin). Previously,

Re: [PATCH 0/10] towards clean leak-checker output

On 5 September 2017 at 21:02, Jeff King wrote: > On Tue, Sep 05, 2017 at 07:50:10PM +0200, Martin Ågren wrote: > >> > That line is the setting of argv0_path, which is a global (and thus >> > shouldn't be marked as leaking). Interestingly, it only happens with >> > -O2.

Re: [PATCH v3 1/3] refs/files-backend: add longer-scoped copy of string to list

On Tue, Sep 05, 2017 at 07:24:18PM +0200, Martin Ågren wrote: > > And from that point of view, doesn't split_head_update() wants a > > similar fix? It attempts to insert "HEAD", makes sure it hasn't > > been inserted and then hangs a new update transaction as its util. > > It is not wrong per-se

Re: Empty directories in Git: Current guidance for historical commits?

On Tue, Sep 05, 2017 at 12:47:43PM +, wafflec...@openmail.cc wrote: > Is just dropping a ".gitignore" or "README" file in the empty directories > during conversion still the most reasonable approach? Yes, I think so. > If so, is there a way > to do this automatically during the conversion

RE: [PATCH] read-cache: fix index corruption with index v4

> From: Thomas Gummerer [mailto:t.gumme...@gmail.com] > Sent: Monday, September 4, 2017 4:58 PM > > ce012deb98 ("read-cache: avoid allocating every ondisk entry when > writing", 2017-08-21) changed the way cache entries are written to the > index file. While previously it wrote the name to an

Re: [PATCH 0/10] towards clean leak-checker output

On Tue, Sep 05, 2017 at 07:50:10PM +0200, Martin Ågren wrote: > > That line is the setting of argv0_path, which is a global (and thus > > shouldn't be marked as leaking). Interestingly, it only happens with > > -O2. Compiling with -O0 works fine. I'm not sure if it's a bug or > >

[PATCH] config: remove git_config_maybe_bool

The function was deprecated in commit 89576613 ("treewide: deprecate git_config_maybe_bool, use git_parse_maybe_bool", 2017-08-07) and has no users. Signed-off-by: Martin Ågren --- ma/parse-maybe-bool has been in master for some time and I cannot find any inflight topics

Re: signing commits using gpg2

Apparently you need to set the GPG_TTY for git to work (I also set the gpg.program so I know it shouldn't /need/ that variable set) https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=840687#10 I'm not sure if there's anything that has or can be done upstream to make this easier (I feel this was a

Re: Empty directories in Git: Current guidance for historical commits?

On 2017-09-05 14:47, wafflec...@openmail.cc wrote: > > Hello, > > Per the FAQ it's clear that Git (by current design) does not track empty > directories:  > https://git.wiki.kernel.org/index.php/GitFaq#Can_I_add_empty_directories.3F > > That's fine and I can fix that for my code going forward

Re: [PATCH 0/10] towards clean leak-checker output

On 5 September 2017 at 15:01, Jeff King wrote: > Using leak-checking tools like valgrind or LSAN is usually not all that > productive with Git, because the output is far from clean. And _most_ of > these are just not interesting, as they're either: > > 1. Real leaks, but ones

Re: [PATCH v3 2/3] refs/files-backend: fix memory leak in lock_ref_for_update

On 5 September 2017 at 10:47, Jeff King wrote: > On Tue, Aug 29, 2017 at 07:18:23PM +0200, Martin Ågren wrote: > >> After the previous patch, none of the functions we call hold on to >> `referent.buf`, so we can safely release the string buffer before >> returning. > > I ended up

Re: [PATCH v3 1/3] refs/files-backend: add longer-scoped copy of string to list

On 5 September 2017 at 12:02, Junio C Hamano wrote: > Martin Ågren writes: > >> On 30 August 2017 at 04:52, Michael Haggerty wrote: >>> v3 looks good to me. Thanks! >>> >>> Reviewed-by: Michael Haggerty >>

Re: [PATCH] hashmap: add API to disable item counting when threaded

On 5 September 2017 at 18:39, Jeff Hostetler wrote: > > > On 9/1/2017 7:50 PM, Jonathan Nieder wrote: >> >> Hi, >> >> Johannes Schindelin wrote: >>> >>> On Wed, 30 Aug 2017, Jeff Hostetler wrote: >> >> This is to address concerns raised by ThreadSanitizer on the

Re: [PATCH 17/34] mailinfo: release strbuf on error return in handle_boundary()

On 31 August 2017 at 19:21, René Scharfe wrote: > Am 30.08.2017 um 20:23 schrieb Martin Ågren: >> On 30 August 2017 at 19:49, Rene Scharfe wrote: >>> Signed-off-by: Rene Scharfe >>> --- >>> mailinfo.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>>

Re: [PATCH] hashmap: add API to disable item counting when threaded

On 9/2/2017 4:05 AM, Jeff King wrote: On Wed, Aug 30, 2017 at 06:59:22PM +, Jeff Hostetler wrote: From: Jeff Hostetler This is to address concerns raised by ThreadSanitizer on the mailing list about threaded unprotected R/W access to map.size with my previous

Re: [Bug] commit-tree shouldn't append an extra newline to commit messages

On Tue, Sep 05, 2017 at 12:57:21PM -0400, Ross Kabus wrote: > On Tue, Sep 5, 2017 at 11:36 AM, Jeff King wrote: > > > So I'd argue that "git commit -F" is doing a reasonable > > thing, and "commit-tree -F" should probably change to match it (because > > it's inconsistent, and

Re: [Bug] commit-tree shouldn't append an extra newline to commit messages

On Tue, Sep 5, 2017 at 11:36 AM, Jeff King wrote: > So I'd argue that "git commit -F" is doing a reasonable > thing, and "commit-tree -F" should probably change to match it (because > it's inconsistent, and because if anything the plumbing commit-tree > should err more on the side

Re: [PATCH] hashmap: add API to disable item counting when threaded

On 9/2/2017 4:17 AM, Jeff King wrote: On Sat, Sep 02, 2017 at 01:31:19AM +0200, Johannes Schindelin wrote: Before anybody can ask for this message to be wrapped in _(...) to be translateable, let me suggest instead to add the prefix "BUG: ". Agreed on both (and Jonathan's suggestion to just

Re: [PATCH] hashmap: add API to disable item counting when threaded

On 9/1/2017 7:50 PM, Jonathan Nieder wrote: Hi, Johannes Schindelin wrote: On Wed, 30 Aug 2017, Jeff Hostetler wrote: This is to address concerns raised by ThreadSanitizer on the mailing list about threaded unprotected R/W access to map.size with my previous "disallow rehash" change

Re: [PATCH] hashmap: add API to disable item counting when threaded

On 9/1/2017 7:31 PM, Johannes Schindelin wrote: Hi Jeff, On Wed, 30 Aug 2017, Jeff Hostetler wrote: From: Jeff Hostetler This is to address concerns raised by ThreadSanitizer on the mailing list about threaded unprotected R/W access to map.size with my previous

Re: [Bug] commit-tree shouldn't append an extra newline to commit messages

On Tue, Sep 05, 2017 at 11:09:01AM -0400, Ross Kabus wrote: > On Sat, Sep 2, 2017 at 4:33 AM, Jeff King wrote: > > > But I am confused by your "inconsistent with git commit porcelain" > > comment. The porcelain git-commit definitely _does_ add a newline if one > > isn't present

Re: [Bug] commit-tree shouldn't append an extra newline to commit messages

On Sat, Sep 2, 2017 at 4:33 AM, Jeff King wrote: > But I am confused by your "inconsistent with git commit porcelain" > comment. The porcelain git-commit definitely _does_ add a newline if one > isn't present (and in fact runs the whole thing through git-stripspace > to clean up

Empty directories in Git: Current guidance for historical commits?

Hello, Per the FAQ it's clear that Git (by current design) does not track empty directories: https://git.wiki.kernel.org/index.php/GitFaq#Can_I_add_empty_directories.3F That's fine and I can fix that for my code going forward via the build system, as needed. However I'm looking to

Re: signing commits using gpg2

shawn wilson venit, vidit, dixit 02.09.2017 23:11: > tl;dr - how do I get git to use gpg2 to sign things? > > I'm using gpg2 (so no agent options are configured but an agent is > running) which is configured w/ a Nitrokey (Pro if it matters): > > % git commit -m "Initial." > >

Re: Donation

Greetings To You, My Name is Mavis wanczyk , the winner of the Power ball jackpot of $ $758.7 million in the AUGUST 24, 2017, My jackpot was a gift from God to me hence my Entire family/foundation has AGREED to do this. My foundation is donating $500,000.00USD to you. please contac

[PATCH 08/10] repository: free fields before overwriting them

It's possible that the repository data may be initialized twice (e.g., after doing a chdir() to the top of the worktree we may have to adjust a relative git_dir path). We should free() any existing fields before assigning to them to avoid leaks. This should be safe, as the fields are set based on

[PATCH 07/10] reset: free allocated tree buffers

We read the tree objects with fill_tree_descriptor(), but never actually free the resulting buffers, causing a memory leak. This isn't a huge deal because we call this code at most twice per program invocation. But it does potentially double our heap usage if you have large root trees. Let's free

[PATCH 09/10] set_git_dir: handle feeding gitdir to itself

Ideally we'd free the existing gitdir field before assigning the new one, to avoid a memory leak. But we can't do so safely because some callers do the equivalent of: set_git_dir(get_git_dir()); We can detect that case as a noop, but there are even more complicated cases like:

[PATCH 10/10] add UNLEAK annotation for reducing leak false positives

It's a common pattern in git commands to allocate some memory that should last for the lifetime of the program and then not bother to free it, relying on the OS to throw it away. This keeps the code simple, and it's fast (we don't waste time traversing structures or calling free at the end of the

[PATCH 06/10] reset: make tree counting less confusing

Depending on whether we're in --keep mode, git-reset may feed one or two trees to unpack_trees(). We start a counter at "1" and then increment it to "2" only for the two-tree case. But that means we must always subtract one to find the correct array slot to fill with each descriptor. Instead,

[PATCH 05/10] config: plug user_config leak

We generate filenames for the user_config ("~/.gitconfig") and the xdg config ("$XDG_CONFIG_HOME/git/config") and then decide which to use by looking at the filesystem. But after selecting one, the unused string is just leaked. This is a tiny leak, but it creates noise in leak-checker output.

[PATCH 04/10] update-index: fix cache entry leak in add_one_file()

When we fail to add the cache entry to the index, we end up just leaking the struct. We should follow the pattern of the early-return above and free it. Signed-off-by: Jeff King --- builtin/update-index.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git

[PATCH 03/10] add: free leaked pathspec after add_files_to_cache()

After run_diff_files, we throw away the rev_info struct, including the pathspec that we copied into it, leaking the memory. this is probably not a big deal in practice. We usually only run this once per process, and the leak is proportional to the pathspec list we're already holding in memory.

[PATCH 02/10] test-lib: set LSAN_OPTIONS to abort by default

We already set ASAN_OPTIONS to abort if it finds any errors. As we start to experiment with LSAN, the leak sanitizer, it's convenient if we give it the same treatment. Note that ASAN is actually a superset of LSAN and can do the leak detection itself. So this only has an effect if you

[PATCH 01/10] test-lib: --valgrind should not override --verbose-log

The --verbose test option cannot be used with test harnesses like "prove". Instead, you must use --verbose-log. Since the --valgrind option implies --verbose, that means that it cannot be used with prove. I.e., this does not work: prove t-basic.sh :: --valgrind You'd think it could be

[PATCH 0/10] towards clean leak-checker output

Using leak-checking tools like valgrind or LSAN is usually not all that productive with Git, because the output is far from clean. And _most_ of these are just not interesting, as they're either: 1. Real leaks, but ones that only be triggered a small, set number of times per program. 2.

[PATCH 18/20] lockfile: update lifetime requirements in documentation

Now that the tempfile system we rely on has loosened the lifetime requirements for storage, we can adjust our documentation to match. Signed-off-by: Jeff King --- lockfile.h | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/lockfile.h

[PATCH 20/20] stop leaking lock structs in some simple cases

Now that it's safe to declare a "struct lock_file" on the stack, we can do so (and avoid an intentional leak). These leaks were found by running t and t0001 under valgrind (though certainly other similar leaks exist and just don't happen to be exercised by those tests). Initializing the

[PATCH 19/20] ref_lock: stop leaking lock_files

Since the tempfile code recently relaxed the rule that tempfile structs (and thus locks) need to hang around forever, we no longer have to leak our lock_file structs. In fact, we don't even need to heap-allocate them anymore, since their lifetime can just match that of the surrounding ref_lock

[PATCH 15/20] tempfile: use list.h for linked list

The tempfile API keeps to-be-cleaned tempfiles in a singly-linked list and never removes items from the list. A future patch would like to start removing items, but removal from a singly linked list is O(n), as we have to walk the list to find the predecessor element. This means that a process

[PATCH 14/20] tempfile: release deactivated strbufs instead of resetting

When a tempfile is deactivated, we reset its strbuf to the empty string, which means we hold onto the memory for later reuse. Since we'd like to move to a system where tempfile structs can actually be freed, deactivating one should drop all resources it is currently using. And thus "release"

[PATCH 17/20] tempfile: auto-allocate tempfiles on heap

The previous commit taught the tempfile code to give up ownership over tempfiles that have been renamed or deleted. That makes it possible to use a stack variable like this: struct tempfile t; create_tempfile(, ...); ... if (!err) rename_tempfile(, ...); else

[PATCH 16/20] tempfile: remove deactivated list entries

Once a "struct tempfile" is added to the global cleanup list, it is never removed. This means that its storage must remain valid for the lifetime of the program. For single-use tempfiles and locks, this isn't a big deal: we just declare the struct static. But for library code which may take

[PATCH 13/20] tempfile: robustify cleanup handler

We may call remove_tempfiles() from an atexit handler, or from a signal handler. In the latter case we must take care to avoid functions which may deadlock if the process is in an unknown state, including looking at any stdio handles (which may be in the middle of doing I/O and locked) or calling

[PATCH 10/20] tempfile: replace die("BUG") with BUG()

Compared to die(), using BUG() triggers abort(). That may give us an actual coredump, which should make it easier to get a stack trace. And since the programming error for these assertions is not in the functions themselves but in their callers, such a stack trace is needed to actually find the

[PATCH 11/20] tempfile: factor out activation

There are a few steps required to "activate" a tempfile struct. Let's pull these out into a function. That saves a few repeated lines now, but more importantly will make it easier to change the activation scheme later. Signed-off-by: Jeff King --- tempfile.c | 18

[PATCH 12/20] tempfile: factor out deactivation

When we deactivate a tempfile, we also have to clean up the "filename" strbuf. Let's pull this out into its own function to keep the logic in one place (which will become more important when a future patch makes it more complicated). Note that we can use the same function when deactivating an

[PATCH 09/20] tempfile: handle NULL tempfile pointers gracefully

The tempfile functions all take pointers to tempfile objects, but do not check whether the argument is NULL. This isn't a big deal in practice, since the lifetime of any tempfile object is defined to last for the whole program. So even if we try to call delete_tempfile() on an already-deleted

[PATCH 08/20] tempfile: prefer is_tempfile_active to bare access

The tempfile code keeps an "active" flag, and we have a number of assertions to make sure that the objects are being used in the right order. Most of these directly check "active" rather than using the is_tempfile_active() accessor. Let's prefer using the accessor, in preparation for it growing

[PATCH 07/20] lockfile: do not rollback lock on failed close

Since the lockfile code is based on the tempfile code, it has some of the same problems, including that close_lock_file() erases the tempfile's filename buf, making it hard for the caller to write a good error message. In practice this comes up less for lockfiles than for straight tempfiles,

[PATCH 04/20] verify_signed_buffer: prefer close_tempfile() to close()

We do a manual close() on the descriptor provided to us by mks_tempfile. But this runs contrary to the advice in tempfile.h, which notes that you should always use close_tempfile(). Otherwise the descriptor may be reused without the tempfile object knowing it, and the later call to

[PATCH 06/20] tempfile: do not delete tempfile on failed close

When close_tempfile() fails, we delete the tempfile and reset the fields of the tempfile struct. This makes it easier for callers to return without cleaning up, but it also makes this common pattern: if (close_tempfile(tempfile)) return error_errno("error closing %s",

[PATCH 05/20] always check return value of close_tempfile

If close_tempfile() encounters an error, then it deletes the tempfile and resets the "struct tempfile". But many code paths ignore the return value and continue to use the tempfile. Instead, we should generally treat this the same as a write() error. Note that in the postimage of some of these

[PATCH 0/20] allowing struct lock_file to be deleted

Since the beginning of time, our code base has had the rule that the storage for a "struct lock_file" can never go out of scope once it has been used. This leads to lots of intentional leaks of these structs. I think it's unlikely that any of the leaks produces truly terrible behavior, simply

[PATCH 03/20] setup_temporary_shallow: move tempfile struct into function

The setup_temporary_shallow() function creates a temporary file, but we never access the tempfile struct outside of the function. This is OK, since it means we'll just clean up the tempfile on exit. But we can simplify the code a bit by moving the global tempfile struct to the only function in

[PATCH 02/20] setup_temporary_shallow: avoid using inactive tempfile

When there are no shallow entries to write, we skip creating the tempfile entirely and try to return the empty string. But we do so by calling get_tempfile_path() on the inactive tempfile object. This will trigger an assertion that kills the program. The bug was introduced by 6e122b449b

[PATCH 01/20] write_index_as_tree: cleanup tempfile on error

If we failed to write our new index file, we rollback our lockfile to remove the temporary index. But if we fail before we even get to the write step (because reading the old index failed), we leave the lockfile in place, which makes no sense. In practice this hasn't been a big deal because

Re: Git in Outreachy round 15?

On Tue, Sep 05, 2017 at 11:19:56AM +0900, Junio C Hamano wrote: > $6,500 is a bargain if we could guide an intern and help set the > course to become a competent open source person. It would be nice > if the intern stays in our community, but I do not even mind if the > investment follows the

Re: Donation

Greetings To You, My Name is Mavis wanczyk , the winner of the Power ball jackpot of $ $758.7 million in the AUGUST 24, 2017, My jackpot was a gift from God to me hence my Entire family/foundation has AGREED to do this. My foundation is donating $500,000.00USD to you. please contac

Re: [PATCH] pkt-line: re-'static'-ify buffer in packet_write_fmt_1()

Jeff King writes: > Overall I suspect that running our cmd_*() functions multiple times in a > single process is already going to cause chaos, because they often are > touching static globals, etc. So it's probably not that big a deal in > practice to add one more variable to the

Re: [PATCH] pkt-line: re-'static'-ify buffer in packet_write_fmt_1()

Jeff King writes: > On Mon, Aug 28, 2017 at 10:52:51AM -0700, Stefan Beller wrote: > >> >> I'm curious, too. I don't think the valgrind setup in our test suite is >> >> great for finding leaks right now. >> >> This discussion comes up every once in a while, >> the last time I was

Re: [PATCH v3 1/3] refs/files-backend: add longer-scoped copy of string to list

Martin Ågren writes: > On 30 August 2017 at 04:52, Michael Haggerty wrote: >> v3 looks good to me. Thanks! >> >> Reviewed-by: Michael Haggerty > > Thank _you_ for very helpful feedback on the earlier versions. > > Martin Yes,

RE: MICROSOFT OUTLOOK

From: Rebecca Lambarth Sent: 05 September 2017 20:42 Subject: MICROSOFT OUTLOOK Welcome to the new outlook web app. The new Outlook Web app is the new home for online self-service and information. please use our secure online portal at

Re: [PATCH v3 1/3] refs/files-backend: add longer-scoped copy of string to list

On Tue, Sep 05, 2017 at 11:03:36AM +0200, Michael Haggerty wrote: > > It feels pretty dirty, though. It would certainly be a bug if we ever > > decided to switch affected_refnames to duplicate its strings. > > > > So given that your solution is only a constant-time factor worse in > > efficiency,

Re: [PATCH v3 1/3] refs/files-backend: add longer-scoped copy of string to list

On Tue, Sep 5, 2017 at 10:45 AM, Jeff King wrote: > On Tue, Aug 29, 2017 at 07:18:22PM +0200, Martin Ågren wrote: > >> [...] >> Rearrange the handling of `referent`, so that we don't add it directly >> to `affected_refnames`. Instead, first just check whether `referent` >> exists

Re: [PATCH v3 2/3] refs/files-backend: fix memory leak in lock_ref_for_update

On Tue, Aug 29, 2017 at 07:18:23PM +0200, Martin Ågren wrote: > After the previous patch, none of the functions we call hold on to > `referent.buf`, so we can safely release the string buffer before > returning. I ended up doing this a little differently in my version: diff --git

Re: [PATCH v3 1/3] refs/files-backend: add longer-scoped copy of string to list

On Tue, Aug 29, 2017 at 07:18:22PM +0200, Martin Ågren wrote: > Observe that split_symref_update() creates a `new_update`-object through > ref_transaction_add_update(), after which `new_update->refname` is a > copy of `referent`. The difference is, this copy will be freed, and it > will be freed

Dear Talented!

Dear Talented, I am Talent Scout For BLUE SKY FILM STUDIO, Present Blue Sky Studio a Film Corporation Located in the United State, is Soliciting for the Right to use Your Photo/Face and Personality as One of the Semi -Major Role/ Character in our Upcoming ANIMATED Stereoscope 3D Movie-The