refs.c:
int close_ref(struct ref_lock *lock)
{
if (close_lock_file(lock->lk))
return -1;
lock->lock_fd = -1;
return 0;
}
When the close() fails, fd is still >= 0, even if the file is closed.
Could it be written like this ?
int close_ref(struct ref_lock *lock)
{
return close_lock_file(lock->lk);
}
=====================
lockfile.c, line 49
* - filename holds the filename of the lockfile
I think we have a strbuf here? (which is a good thing),
should we write:
* - strbuf filename holds the filename of the lockfile
----------
(and at some places filename[0] is mentioned,
should that be filename.buf[0] ?)
=========================
int commit_lock_file(struct lock_file *lk)
{
static struct strbuf result_file = STRBUF_INIT;
int err;
if (lk->fd >= 0 && close_lock_file(lk))
return -1;
##What happens if close() fails and close_lock_file() returns != 0?
##Is the lk now in a good shaped state?
I think the file which had been open by lk->fd is in an unkown state,
and should not be used for anything.
When I remember it right, an error on close() can mean "the file could not
be written and closed as expected, it can be truncated or corrupted.
This can happen on a network file system like NFS, or probably even other FS.
For me the failing of close() means I smell a need for a rollback.
if (!lk->active)
die("BUG: attempt to commit unlocked object");
/* remove ".lock": */
strbuf_add(&result_file, lk->filename.buf,
lk->filename.len - LOCK_SUFFIX_LEN);
err = rename(lk->filename.buf, result_file.buf);
strbuf_reset(&result_file);
if (err)
return -1;
lk->active = 0;
strbuf_reset(&lk->filename);
return 0;
}
================
Please treat my comments more than questions rather than answers,
thanks for an interesting reading
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
