Michael Haggerty <mhagger@xxxxxxxxxxxx> writes: > Document a couple more functions and the flags argument as used by > hold_lock_file_for_update() and hold_lock_file_for_append(). > Reorganize the document to make it more accessible. > > Helped-by: Jonathan Nieder <jrnieder@xxxxxxxxx> > Signed-off-by: Michael Haggerty <mhagger@xxxxxxxxxxxx> > --- > Documentation/technical/api-lockfile.txt | 199 ++++++++++++++++++++++--------- > 1 file changed, 145 insertions(+), 54 deletions(-) Nicely done. > +* Mutual exclusion and atomic file updates. When we want to change a > + file, we create a lockfile `<filename>.lock`, write the new file > + contents into it, and then rename the lockfile to its final > + destination `<filename>`. We create the `<filename>.lock` file with > + `O_CREAT|O_EXCL` so that we can notice and fail if somebody else has > + already locked the file. You may want to say then atomically rename the lockfile to its final destination to commit the changes and unlock the file. here; that way, your mention of "commit" in the next paragraph would become easier to understand. > +* Automatic cruft removal. If the program exits after we lock a file > + but before the changes have been committed, we want to make sure > + that we remove the lockfile. > +Calling sequence > +---------------- > + > +The caller: > + > +* Allocates a variable `struct lock_file lock` in the bss section or > + heap, initialized to zeros. It cannot be an auto variable allocated > + on the stack. Because the `lock_file` structure is used in an > + `atexit(3)` handler, its storage has to stay throughout the life of > + the program, even after the file changes have been committed or > + rolled back. It is easier to read if you pushed the second sentence (which is a rephrase of the first one) and third sentence (which explains why the second sentence is true) out of line as a side-note, I think, or probably remove them from here (see below). > +* Attempts to create a lockfile by passing that variable and the path > + of the final destination (e.g. `$GIT_DIR/index`) to > + `hold_lock_file_for_update` or `hold_lock_file_for_append`. > + > +* Writes new content for the destination file by writing to the file > + descriptor returned by those functions (also available via > + `lock->fd`). > + > +When finished writing, the caller can: > + > +* Close the file descriptor and rename the lockfile to its final > + destination by calling `commit_lock_file`. > + > +* Close the file descriptor and remove the lockfile by calling > + `rollback_lock_file`. > + > +* Close the file descriptor without removing or renaming the lockfile > + by calling `close_lock_file`, and later call `commit_lock_file` or > + `rollback_lock_file`. > + > +At this point, the `lock_file` object must not be freed or altered, > +because it is still registered in the `lock_file_list`. However, it > +may be reused; just pass it to another call of > +`hold_lock_file_for_update` or `hold_lock_file_for_append`. It feels a bit conflicting between "must not be freed or ALTERED" and "it may be REUSED". Drop "or altered" to reduce confusion? And this repeats the third sentence I suggested to remove from the first paragraph (above 'see below' refers here). Is it allowed to tell the name of this lock_file to other people and let them read from it? The answer is yes but it is not obvious from this description. Also mention how the above interact with reopen_lock_file() here? > +If the program exits before you have called one of `commit_lock_file`, > +`rollback_lock_file`, or `close_lock_file`, an `atexit(3)` handler > +will close and remove the lockfile, essentially rolling back any > +uncommitted changes. s/essentially //; > +Error handling > +-------------- > + > +The `hold_lock_file_*` functions return a file descriptor on success > +or -1 on failure (unless `LOCK_DIE_ON_ERROR` is used; see below). On > +errors, `errno` describes the reason for failure. Errors can be > +handled by passing `errno` to one of the following helper functions: s/handled by/reported by/; probably. None of these will help you "handle" errors in the sense to (attempt to) recover from it. > +unable_to_lock_message:: > + > + Append an appropriate error message to a `strbuf`. > + > +unable_to_lock_error:: > + > + Emit an appropriate error message using `error()`. > + > +unable_to_lock_die:: > + > + Emit an appropriate error message and `die()`. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html