summaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAgeFilesLines
* contacts: add a Makefile to generate docs and installss/contrib-subtree-contactsSebastian Schuberth2014-10-152-0/+74
| | | | | | | Also add a gitignore file for generated files. Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* subtree: add an install-html targetSebastian Schuberth2014-10-152-3/+9
| | | | | | | Also adjust ignore rules accordingly. Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Update draft release notes to 2.2Junio C Hamano2014-10-141-0/+21
| | | | Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'bc/asciidoc-pretty-formats-fix'Junio C Hamano2014-10-141-1/+1
|\ | | | | | | | | * bc/asciidoc-pretty-formats-fix: Documentation: fix misrender of pretty-formats in Asciidoctor
| * Documentation: fix misrender of pretty-formats in Asciidoctorbc/asciidoc-pretty-formats-fixbrian m. carlson2014-10-081-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | Neither the AsciiDoc nor the Asciidoctor documentation specify whether the same number of delimiter characters must be used to end a block as to begin it, although both sets of documentation show exactly matching pairs. AsciiDoc allows mismatches, but AsciiDoctor apparently does not. Adjust the pretty formats documentation to use matching pairs to prevent a misrendering where the remainder of the document was rendered as a listing block. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'rs/plug-leak-in-bundle'Junio C Hamano2014-10-141-7/+10
|\ \ | | | | | | | | | | | | * rs/plug-leak-in-bundle: bundle: plug minor memory leak in is_tag_in_date_range()
| * | bundle: plug minor memory leak in is_tag_in_date_range()rs/plug-leak-in-bundleRené Scharfe2014-10-071-7/+10
| | | | | | | | | | | | | | | | | | | | | | | | | | | Free the buffer returned by read_sha1_file() even if no valid tagger line is found. Signed-off-by: Rene Scharfe <l.s.r@web.de> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | Merge branch 'rs/more-uses-of-skip-prefix'Junio C Hamano2014-10-1410-75/+69
|\ \ \ | | | | | | | | | | | | | | | | * rs/more-uses-of-skip-prefix: use skip_prefix() to avoid more magic numbers
| * | | use skip_prefix() to avoid more magic numbersrs/more-uses-of-skip-prefixRené Scharfe2014-10-0710-75/+69
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Continue where ae021d87 (use skip_prefix to avoid magic numbers) left off and use skip_prefix() in more places for determining the lengths of prefix strings to avoid using dependent constants and other indirect methods. Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | Merge branch 'rs/mailsplit'Junio C Hamano2014-10-141-1/+0
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | * rs/mailsplit: mailsplit: remove unnecessary unlink(2) call
| * | | | mailsplit: remove unnecessary unlink(2) callrs/mailsplitRené Scharfe2014-10-071-1/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The output file hasn't been created at this point, yet, so there is no need to delete it when exiting early. Suggested-by: Jeff King <peff@peff.net> Signed-off-by: Rene Scharfe <l.s.r@web.de> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | Merge branch 'rs/sha1-array-test'Junio C Hamano2014-10-145-2/+130
|\ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * rs/sha1-array-test: sha1-lookup: handle duplicates in sha1_pos() sha1-array: add test-sha1-array and basic tests
| * | | | | sha1-lookup: handle duplicates in sha1_pos()rs/sha1-array-testRené Scharfe2014-10-012-2/+20
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If the first 18 bytes of the SHA1's of all entries are the same then sha1_pos() dies and reports that the lower and upper limits of the binary search were the same that this wasn't supposed to happen. This is wrong because the remaining two bytes could still differ. Furthermore: It wouldn't be a problem if they actually were the same, i.e. if all entries have the same SHA1. The code already handles duplicates just fine. Simply remove the erroneous check. Signed-off-by: Rene Scharfe <l.s.r@web.de> Acked-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | sha1-array: add test-sha1-array and basic testsRené Scharfe2014-10-014-0/+110
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Helped-by: Jeff King <peff@peff.net> Helped-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Rene Scharfe <l.s.r@web.de> Acked-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | Merge branch 'mh/lockfile-stdio'Junio C Hamano2014-10-145-39/+71
|\ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * mh/lockfile-stdio: commit_packed_refs(): reimplement using fdopen_lock_file() dump_marks(): reimplement using fdopen_lock_file() fdopen_lock_file(): access a lockfile using stdio
| * | | | | | commit_packed_refs(): reimplement using fdopen_lock_file()mh/lockfile-stdioMichael Haggerty2014-10-011-4/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | dump_marks(): reimplement using fdopen_lock_file()Michael Haggerty2014-10-011-19/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | fdopen_lock_file(): access a lockfile using stdioMichael Haggerty2014-10-013-16/+68
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add a new function, fdopen_lock_file(), which returns a FILE pointer open to the lockfile. If a stream is open on a lock_file object, it is closed using fclose() on commit, rollback, or close_lock_file(). This change will allow callers to use stdio to write to a lockfile without having to muck around in the internal representation of the lock_file object (callers will be rewritten in upcoming commits). Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | Merge branch 'mh/lockfile'Junio C Hamano2014-10-1436-260/+510
|\ \ \ \ \ \ \ | |/ / / / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The lockfile API and its users have been cleaned up. * mh/lockfile: (38 commits) lockfile.h: extract new header file for the functions in lockfile.c hold_locked_index(): move from lockfile.c to read-cache.c hold_lock_file_for_append(): restore errno before returning get_locked_file_path(): new function lockfile.c: rename static functions lockfile: rename LOCK_NODEREF to LOCK_NO_DEREF commit_lock_file_to(): refactor a helper out of commit_lock_file() trim_last_path_component(): replace last_path_elm() resolve_symlink(): take a strbuf parameter resolve_symlink(): use a strbuf for internal scratch space lockfile: change lock_file::filename into a strbuf commit_lock_file(): use a strbuf to manage temporary space try_merge_strategy(): use a statically-allocated lock_file object try_merge_strategy(): remove redundant lock_file allocation struct lock_file: declare some fields volatile lockfile: avoid transitory invalid states git_config_set_multivar_in_file(): avoid call to rollback_lock_file() dump_marks(): remove a redundant call to rollback_lock_file() api-lockfile: document edge cases commit_lock_file(): rollback lock file on failure to rename ...
| * | | | | | lockfile.h: extract new header file for the functions in lockfile.cmh/lockfileMichael Haggerty2014-10-0135-83/+118
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Move the interface declaration for the functions in lockfile.c from cache.h to a new file, lockfile.h. Add #includes where necessary (and remove some redundant includes of cache.h by files that already include builtin.h). Move the documentation of the lock_file state diagram from lockfile.c to the new header file. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | hold_locked_index(): move from lockfile.c to read-cache.cMichael Haggerty2014-10-012-8/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | lockfile.c contains the general API for locking any file. Code specifically about the index file doesn't belong here. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | hold_lock_file_for_append(): restore errno before returningMichael Haggerty2014-10-011-1/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Callers who don't pass LOCK_DIE_ON_ERROR might want to examine errno to see what went wrong, so restore errno before returning. In fact this function only has one caller, add_to_alternates_file(), and it *does* use LOCK_DIE_ON_ERROR, but, you know, think of future generations. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | get_locked_file_path(): new functionMichael Haggerty2014-10-014-3/+16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add a function to return the path of the file that is locked by a lock_file object. This reduces the knowledge that callers have to have about the lock_file layout. Suggested-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | lockfile.c: rename static functionsMichael Haggerty2014-10-011-5/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * remove_lock_file() -> remove_lock_files() * remove_lock_file_on_signal() -> remove_lock_files_on_signal() Suggested-by: Torsten Bögershausen <tboegi@web.de> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | lockfile: rename LOCK_NODEREF to LOCK_NO_DEREFMichael Haggerty2014-10-014-5/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This makes it harder to misread the name as LOCK_NODE_REF. Suggested-by: Torsten Bögershausen <tboegi@web.de> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | commit_lock_file_to(): refactor a helper out of commit_lock_file()Michael Haggerty2014-10-014-38/+50
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | commit_locked_index(), when writing to an alternate index file, duplicates (poorly) the code in commit_lock_file(). And anyway, it shouldn't have to know so much about the internal workings of lockfile objects. So extract a new function commit_lock_file_to() that does the work common to the two functions, and call it from both commit_lock_file() and commit_locked_index(). Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | trim_last_path_component(): replace last_path_elm()Michael Haggerty2014-10-011-22/+16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Rewrite last_path_elm() to take a strbuf parameter and to trim off the last path name element in place rather than returning a pointer to the beginning of the last path name element. This simplifies the function a bit and makes it integrate better with its caller, which is now also strbuf-based. Rename the function accordingly and a bit less tersely. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | resolve_symlink(): take a strbuf parameterMichael Haggerty2014-10-011-35/+22
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Change resolve_symlink() to take a strbuf rather than a string as parameter. This simplifies the code and removes an arbitrary pathname length restriction. It also means that lock_file's filename field no longer needs to be initialized to a large size. Helped-by: Torsten Bögershausen <tboegi@web.de> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | resolve_symlink(): use a strbuf for internal scratch spaceMichael Haggerty2014-10-011-21/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Aside from shortening and simplifying the code, this removes another place where the path name length is arbitrarily limited. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | lockfile: change lock_file::filename into a strbufMichael Haggerty2014-10-018-52/+47
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | For now, we still make sure to allocate at least PATH_MAX characters for the strbuf because resolve_symlink() doesn't know how to expand the space for its return value. (That will be fixed in a moment.) Another alternative would be to just use a strbuf as scratch space in lock_file() but then store a pointer to the naked string in struct lock_file. But lock_file objects are often reused. By reusing the same strbuf, we can avoid having to reallocate the string most times when a lock_file object is reused. Helped-by: Torsten Bögershausen <tboegi@web.de> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | commit_lock_file(): use a strbuf to manage temporary spaceMichael Haggerty2014-10-011-5/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Avoid relying on the filename length restrictions that are currently checked by lock_file(). Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | try_merge_strategy(): use a statically-allocated lock_file objectMichael Haggerty2014-10-011-7/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Even the one lockfile object needn't be allocated each time the function is called. Instead, define one statically-allocated lock_file object and reuse it for every call. Suggested-by: Jeff King <peff@peff.net> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | try_merge_strategy(): remove redundant lock_file allocationMichael Haggerty2014-10-011-1/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | By the time the "if" block is entered, the lock_file instance from the main function block is no longer in use, so re-use that one instead of allocating a second one. Note that the "lock" variable in the "if" block shadowed the "lock" variable at function scope, so the only change needed is to remove the inner definition. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | struct lock_file: declare some fields volatileMichael Haggerty2014-10-012-4/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The function remove_lock_file_on_signal() is used as a signal handler. It is not realistic to make the signal handler conform strictly to the C standard, which is very restrictive about what a signal handler is allowed to do. But let's increase the likelihood that it will work: The lock_file_list global variable and several fields from struct lock_file are used by the signal handler. Declare those values "volatile" to (1) force the main process to write the values to RAM promptly, and (2) prevent updates to these fields from being reordered in a way that leaves an opportunity for a jump to the signal handler while the object is in an inconsistent state. We don't mark the filename field volatile because that would prevent the use of strcpy(), and it is anyway unlikely that a compiler re-orders a strcpy() call across other expressions. So in practice it should be possible to get away without "volatile" in the "filename" case. Suggested-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | lockfile: avoid transitory invalid statesMichael Haggerty2014-10-013-11/+28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Because remove_lock_file() can be called any time by the signal handler, it is important that any lock_file objects that are in the lock_file_list are always in a valid state. And since lock_file objects are often reused (but are never removed from lock_file_list), that means we have to be careful whenever mutating a lock_file object to always keep it in a well-defined state. This was formerly not the case, because part of the state was encoded by setting lk->filename to the empty string vs. a valid filename. It is wrong to assume that this string can be updated atomically; for example, even strcpy(lk->filename, value) is unsafe. But the old code was even more reckless; for example, strcpy(lk->filename, path); if (!(flags & LOCK_NODEREF)) resolve_symlink(lk->filename, max_path_len); strcat(lk->filename, ".lock"); During the call to resolve_symlink(), lk->filename contained the name of the file that was being locked, not the name of the lockfile. If a signal were raised during that interval, then the signal handler would have deleted the valuable file! We could probably continue to use the filename field to encode the state by being careful to write characters 1..N-1 of the filename first, and then overwrite the NUL at filename[0] with the first character of the filename, but that would be awkward and error-prone. So, instead of using the filename field to determine whether the lock_file object is active, add a new field "lock_file::active" for this purpose. Be careful to set this field only when filename really contains the name of a file that should be deleted on cleanup. Helped-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | git_config_set_multivar_in_file(): avoid call to rollback_lock_file()Michael Haggerty2014-10-011-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | After commit_lock_file() is called, then the lock_file object is necessarily either committed or rolled back. So there is no need to call rollback_lock_file() again in either of these cases. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | dump_marks(): remove a redundant call to rollback_lock_file()Michael Haggerty2014-10-011-3/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When commit_lock_file() fails, it now always calls rollback_lock_file() internally, so there is no need to call that function here. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | api-lockfile: document edge casesMichael Haggerty2014-10-011-6/+14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * Document the behavior of commit_lock_file() when it fails, namely that it rolls back the lock_file object and sets errno appropriately. * Document the behavior of rollback_lock_file() when called for a lock_file object that has already been committed or rolled back, namely that it is a NOOP. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | commit_lock_file(): rollback lock file on failure to renameMichael Haggerty2014-10-011-1/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If rename() fails, call rollback_lock_file() to delete the lock file (in case it is still present) and reset the filename field to the empty string so that the lockfile object is left in a valid state. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | close_lock_file(): if close fails, roll backMichael Haggerty2014-10-012-13/+22
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If closing an open lockfile fails, then we cannot be sure of the contents of the lockfile, so there is nothing sensible to do but delete it. This change also insures that the lock_file object is left in a defined state in this error path (namely, unlocked). The only caller that is ultimately affected by this change is try_merge_strategy() -> write_locked_index(), which can call close_lock_file() via various execution paths. This caller uses a static lock_file object which previously could have been reused after a failed close_lock_file() even though it was still in locked state. This change causes the lock_file object to be unlocked on failure, thus fixing this error-handling path. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | commit_lock_file(): die() if called for unlocked lockfile objectMichael Haggerty2014-10-012-1/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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 before continuing, do a consistency check that the lock_file object really is locked. Helped-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | commit_lock_file(): inline temporary variableMichael Haggerty2014-10-011-3/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | remove_lock_file(): call rollback_lock_file()Michael Haggerty2014-10-011-6/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It does just what we need. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | lock_file(): exit early if lockfile cannot be openedMichael Haggerty2014-10-011-12/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This is a bit easier to read than the old version, which nested part of the non-error code in an "if" block. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Reviewed-by: Ronnie Sahlberg <sahlberg@google.com> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | prepare_index(): declare return value to be (const char *)Michael Haggerty2014-10-011-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Declare the return value to be const to make it clear that we aren't giving callers permission to write over the string that it points at. (The return value is the filename field of a struct lock_file, which can be used by a signal handler at any time and therefore shouldn't be tampered with.) Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | delete_ref_loose(): don't muck around in the lock_file's filenameMichael Haggerty2014-10-011-6/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It's bad manners. Especially since there could be a signal during the call to unlink_or_warn(), in which case the signal handler will see the wrong filename and delete the reference file, leaving the lockfile behind. So make our own copy to work with. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | cache.h: define constants LOCK_SUFFIX and LOCK_SUFFIX_LENMichael Haggerty2014-10-013-8/+14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | There are a few places that use these values, so define constants for them. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | lockfile.c: document the various states of lock_file objectsMichael Haggerty2014-10-011-0/+42
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Document the valid states of lock_file objects, how they get into each state, and how the state is encoded in the object's fields. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Reviewed-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | lock_file(): always initialize and register lock_file objectMichael Haggerty2014-10-011-9/+16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The purpose of this change is to make the state diagram for lock_file objects simpler and deterministic. If locking fails, lock_file() sometimes leaves the lock_file object partly initialized, but sometimes not. It sometimes registers the object in lock_file_list, but sometimes not. This makes the state diagram for lock_file objects effectively indeterministic and hard to reason about. A future patch will also change the filename field into a strbuf, which needs more involved initialization, so it will become even more important that the state of a lock_file object is well-defined after a failed attempt to lock. The ambiguity doesn't currently have any ill effects, because lock_file objects cannot be removed from the lock_file_list anyway. But to make it easier to document and reason about the code, make this behavior consistent: *always* initialize the lock_file object and *always* register it in lock_file_list the first time it is used, regardless of whether an error occurs. While we're at it, make sure that all of the lock_file fields are initialized to values appropriate for an unlocked object; the caller is only responsible for making sure that on_list is set to zero before the first time it is used. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | hold_lock_file_for_append(): release lock on errorsMichael Haggerty2014-10-011-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If there is an error copying the old contents to the lockfile, roll back the lockfile before exiting so that the lockfile is not held until process cleanup. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>