Thread (37 messages) 37 messages, 3 authors, 2016-06-15

Re: [PATCH v2 17/25] commit_lock_file(): make committing an unlocked lockfile a NOP

flat view

From: Michael Haggerty <hidden>
Date: 2016-06-15 23:00:41

On 04/07/2014 09:31 PM, Jeff King wrote:
On Mon, Apr 07, 2014 at 01:33:59AM +0200, Michael Haggerty wrote:
quoted
It was previously a bug to call commit_lock_file() with a lock_file
object that was not active (an illegal access would happen within the
function).  It was presumably never done, but this would be an easy
programming error to overlook.  So guard the file-renaming code with
an if statement to change committing an unlocked file into a NOP.
[...]
Yeah, I would have expected a die("BUG") here.

I think it is worth making it a fatal mistake and catching it. Rolling
back an uninitialized lockfile is probably OK; we are canceling an
operation that never started. But committing a lockfile that we didn't
actually fill out could be a sign of a serious error, and we may be
propagating a bogus success code. E.g., imagine that receive-pack claims
to have written your ref, but actually commit_lock_file was a silent
NOP. I'd much rather have it die loudly so we can track down the case.
OK, I will change this to a fatal error in v3.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help