Re: [PATCH 2/5] refs.c: do not die if locking fails in `write_pseudoref()`

2018-05-08 Thread Jeff King
On Mon, May 07, 2018 at 05:12:27AM -0600, David Turner wrote: > >Right. After commit 076aa2cbda (tempfile: auto-allocate tempfiles on > >heap, 2017-09-05) this is safe though. Quite a few locks have already > >been moved to the stack, e.g., in 14bca6c63c (sequencer: make lockfiles > >non-static,

Re: [PATCH 2/5] refs.c: do not die if locking fails in `write_pseudoref()`

2018-05-07 Thread Stefan Beller
+cc Michael, who did extensive work in the refs.c code. On Sun, May 6, 2018 at 7:10 AM, Martin Ågren wrote: > If we could not take the lock, we add an error to the `strbuf err` and > return. However, this code is dead. The reason is that we take the lock > using

Re: [PATCH 2/5] refs.c: do not die if locking fails in `write_pseudoref()`

2018-05-07 Thread David Turner
On May 6, 2018 9:56:31 AM MDT, "Martin Ågren" wrote: >On 6 May 2018 at 17:48, David Turner wrote: >> On Sun, 2018-05-06 at 16:10 +0200, Martin Ågren wrote: >>> While at it, make the lock non-static. > >> Re making the lock static, I wonder about the

Re: [PATCH 2/5] refs.c: do not die if locking fails in `write_pseudoref()`

2018-05-06 Thread Martin Ågren
On 6 May 2018 at 17:48, David Turner wrote: > On Sun, 2018-05-06 at 16:10 +0200, Martin Ågren wrote: >> While at it, make the lock non-static. > Re making the lock static, I wonder about the following case: > > if (read_ref(pseudoref, _old_oid)) > > die("could not read

Re: [PATCH 2/5] refs.c: do not die if locking fails in `write_pseudoref()`

2018-05-06 Thread David Turner
Re making the lock static, I wonder about the following case: if (read_ref(pseudoref, _old_oid)) die("could not read ref '%s'", pseudoref); I think this calls exit(), and then atexit tries to clean up the lock files. But since lock is no longer static,

[PATCH 2/5] refs.c: do not die if locking fails in `write_pseudoref()`

2018-05-06 Thread Martin Ågren
If we could not take the lock, we add an error to the `strbuf err` and return. However, this code is dead. The reason is that we take the lock using `LOCK_DIE_ON_ERROR`. Drop the flag to allow our more gentle error-handling to actually kick in. We could instead just drop the dead code and die