Re: [PATCH v2 18/25] lockfile: avoid transitory invalid states

Am 4/7/2014 1:34, schrieb Michael Haggerty: > 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 fr

[PATCH v2 25/25] trim_last_path_elm(): replace last_path_elm()

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 also strbuf-based. Rena

[PATCH v2 22/25] Change lock_file::filename into a strbuf

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 sto

[PATCH v2 13/25] write_packed_entry_fn(): convert cb_data into a (const int *)

This makes it obvious that we have no plans to change the integer pointed to, which is actually the fd field from a struct lock_file. Signed-off-by: Michael Haggerty --- refs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/refs.c b/refs.c index 64ea2f0..cd471ab 100644 --- a

[PATCH v2 10/25] lockfile: define a constant LOCK_SUFFIX_LEN

Signed-off-by: Michael Haggerty --- lockfile.c | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lockfile.c b/lockfile.c index 2438430..1bd0ae1 100644 --- a/lockfile.c +++ b/lockfile.c @@ -178,14 +178,17 @@ static char *resolve_symlink(char *p, size_t s) retu

[PATCH v2 23/25] resolve_symlink(): use a strbuf for internal scratch space

Aside from shortening and simplifying the code, this removes another place where the path name length is arbitrarily limited. Signed-off-by: Michael Haggerty --- lockfile.c | 33 - 1 file changed, 12 insertions(+), 21 deletions(-) diff --git a/lockfile.c b/lockfi

[PATCH v2 11/25] delete_ref_loose(): don't muck around in the lock_file's filename

It's bad manners. Especially since, if unlink_or_warn() failed, the memory wasn't restored to its original contents. So make our own copy to work with. Signed-off-by: Michael Haggerty --- refs.c | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/refs.c b/refs.c

[PATCH v2 21/25] commit_lock_file(): use a strbuf to manage temporary space

Avoid relying on the filename length restrictions that are currently checked by lock_file(). Signed-off-by: Michael Haggerty --- lockfile.c | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/lockfile.c b/lockfile.c index b955cca..bf018b5 100644 --- a/lockfile.c +++

[PATCH v2 15/25] remove_lock_file(): call rollback_lock_file()

It does just what we need. Signed-off-by: Michael Haggerty --- lockfile.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/lockfile.c b/lockfile.c index 0879214..f6bee35 100644 --- a/lockfile.c +++ b/lockfile.c @@ -69,12 +69,8 @@ static void remove_lock_file(void)

[PATCH v2 01/25] api-lockfile: expand the documentation

Document a couple more functions and the flags argument as used by hold_lock_file_for_update() and hold_lock_file_for_append(). Signed-off-by: Michael Haggerty --- Documentation/technical/api-lockfile.txt | 36 +--- 1 file changed, 33 insertions(+), 3 deletions(-) di

[PATCH v2 09/25] lockfile.c: document the various states of lock_file objects

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 --- lockfile.c | 52 1 file changed, 52 insertions(+) diff --git a/lockfile.c b

[PATCH v2 06/25] hold_lock_file_for_append(): release lock on errors

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 --- lockfile.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lockfile.c b/lockfil

[PATCH v2 18/25] lockfile: avoid transitory invalid states

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

[PATCH v2 05/25] lockfile: unlock file if lockfile permissions cannot be adjusted

If the call to adjust_shared_perm() fails, lock_file returns -1, which to the caller looks like any other failure to lock the file. So in this case, roll back the lockfile before returning so that the lock file is deleted immediately and the lockfile object is left in a predictable state (namely,

[PATCH v2 03/25] rollback_lock_file(): do not clear filename redundantly

It is only necessary to clear the lock_file's filename field if it was not already clear. Signed-off-by: Michael Haggerty --- lockfile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lockfile.c b/lockfile.c index d4bfb3f..7701267 100644 --- a/lockfile.c +++ b/lockfile.c @@

[PATCH v2 24/25] resolve_symlink(): take a strbuf parameter

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. Signed-off-by: Michael Haggerty --- lockf

[PATCH v2 14/25] lock_file(): exit early if lockfile cannot be opened

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 --- lockfile.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lockfile.c b/lockfile.c index 1bd0ae1..0879214 100644

[PATCH v2 12/25] prepare_index(): declare return value to be (const char *)

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.) Sig

[PATCH v2 04/25] rollback_lock_file(): set fd to -1

When rolling back the lockfile, call close_lock_file() so that the lock_file's fd field gets set back to -1. This keeps the lock_file object in a valid state, which is important because these objects are allowed to be reused. Signed-off-by: Michael Haggerty --- lockfile.c | 2 +- 1 file changed

[PATCH v2 08/25] struct lock_file: replace on_list field with flags field

This makes space for other bits, which will arrive soon. Signed-off-by: Michael Haggerty --- cache.h| 2 +- lockfile.c | 9 +++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/cache.h b/cache.h index 3a873a4..59ce5cd 100644 --- a/cache.h +++ b/cache.h @@ -537,7 +537,7 @@

[PATCH v2 02/25] unable_to_lock_die(): rename function from unable_to_lock_index_die()

This function is used for other things besides the index, so rename it accordingly. Suggested-by: Jeff King Signed-off-by: Michael Haggerty --- builtin/update-index.c | 2 +- cache.h| 2 +- lockfile.c | 6 +++--- refs.c | 2 +- 4 files changed, 6 inse

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

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 chang

[PATCH v2 16/25] commit_lock_file(): inline temporary variable

Signed-off-by: Michael Haggerty --- lockfile.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lockfile.c b/lockfile.c index f6bee35..47762c6 100644 --- a/lockfile.c +++ b/lockfile.c @@ -297,12 +297,14 @@ int close_lock_file(struct lock_file *lk) int commit_lock_file(

[PATCH v2 19/25] try_merge_strategy(): remove redundant lock_file allocation

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 rem

[PATCH v2 20/25] try_merge_strategy(): use a statically-allocated lock_file object

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 Signed-off-by: Michael Haggerty --- builtin/merge.c | 14 +++--- 1 file changed, 7 insertio

[PATCH v2 07/25] lock_file(): always add lock_file object to lock_file_list

It used to be that if locking failed, lock_file() usually did not register the lock_file object in lock_file_list but sometimes it did. This confusion was compounded if lock_file() was called via hold_lock_file_for_append(), which has its own failure modes. The ambiguity didn't have any ill effect

[PATCH v2 00/25] Lockfile correctness and refactoring

This is a second attempt at renovating the lock file code. Thanks to Peff, Junio, Torsten, and Eric for their helpful reviews of v1. v1 of this patch series [1] did some refactoring and then added a new feature to the lock_file API: the ability to activate a new version of a locked file while ret

Re: What's cooking in git.git (Apr 2014, #01; Fri, 4)

"Junio C Hamano" wrote in message news:xmqq4n28q0ad@gitster.dls.corp.google.com... Here are the topics that have been cooking. ... * po/everyday-doc (2014-01-27) 1 commit - Make 'git help everyday' work This may make the said command to emit something, but the source is not meant to b

[PATCH] describe: rewrite name_rev() iteratively

The "git describe --contains" command uses the name_rev() function which is currently a recursive function. This causes a Stack Overflow when the history is large enough. Rewrite name_rev iteratively using a stack on the heap. This slightly reduces performance due to the extra operations on the he

Segmentation fault on Mac OS Mavericks 10.9.2

I tried to push one of the repos with git 1.9.0 and 1.9.1 and I got *Segmentation fault 11* Crash report for 1.9.1 Crash report for 1.9.0 It works with 1.8.5.5 Sorry I'm not sure if this has already been reported... not 'capable' enou

Re: [PATCH 18/22] lockfile: also keep track of the filename of the file being locked

On 04/02/2014 07:19 PM, Junio C Hamano wrote: > Michael Haggerty writes: > >> This reduces the amount of string editing gyrations and makes it >> unnecessary for callers to know how to derive the filename from the >> lock_filename. > > Hmph. > > Is it worth duplicating the whole thing? If you

Re: [PATCH 13/22] config: change write_error() to take a (struct lock_file *) argument

On 04/02/2014 08:58 AM, Torsten Bögershausen wrote: > On 04/01/2014 05:58 PM, Michael Haggerty wrote: >> Reduce the amount of code that has to know about the lock_file's >> filename field. >> >> Signed-off-by: Michael Haggerty >> --- >> config.c | 10 +- >> 1 file changed, 5 insertions(

Re: [PATCH 05/22] lockfile: unlock file if lockfile permissions cannot be adjusted

On 04/02/2014 08:47 AM, Torsten Bögershausen wrote: > [] > > diff --git a/lockfile.c b/lockfile.c > index c1af65b..1928e5e 100644 > --- a/lockfile.c > +++ b/lockfile.c > @@ -148,9 +148,11 @@ static int lock_file(struct lock_file *lk, const > char *path, int flags) > lock_file_list = l

Re: [PATCH 07/22] lock_file(): always add lock_file object to lock_file_list

On 04/01/2014 10:16 PM, Jeff King wrote: > On Tue, Apr 01, 2014 at 05:58:15PM +0200, Michael Haggerty wrote: > >> diff --git a/lockfile.c b/lockfile.c >> index e679e4c..c989f6c 100644 >> --- a/lockfile.c >> +++ b/lockfile.c >> @@ -130,6 +130,22 @@ static int lock_file(struct lock_file *lk, const c

Re: [PATCH 04/22] rollback_lock_file(): set fd to -1

On 04/02/2014 06:58 PM, Junio C Hamano wrote: > Jeff King writes: > >> On Tue, Apr 01, 2014 at 05:58:12PM +0200, Michael Haggerty wrote: >> >>> When rolling back the lockfile, call close_lock_file() so that the >>> lock_file's fd field gets set back to -1. This could help prevent >>> confusion i

Re: [PATCH v2 18/19] tree-diff: rework diff_tree() to generate diffs for multiparent cases as well

Junio, First of all thanks a lot for reviewing this patch. I'll reply inline with corrected version attached in the end. On Fri, Apr 04, 2014 at 11:42:39AM -0700, Junio C Hamano wrote: > Kirill Smelkov writes: > > > +extern > > +struct combine_diff_path *diff_tree_paths( > > These two on the s

Re: [RFC] git submodule split

On Sun, Apr 06 2014, Jens Lehmann wrote: > Am 02.04.2014 23:52, schrieb Michal Sojka: >> Hello, >> >> I needed to convert a subdirectory of a repo to a submodule and have the >> histories of both repos linked together. I found that this was discussed >> few years back [1], but the code seemed quit

Re: [PATCH v2.1] commit: add --ignore-submodules[=] parameter

Am 02.04.2014 21:56, schrieb Ronald Weiss: > On 2. 4. 2014 20:53, Jens Lehmann wrote: >> Am 01.04.2014 23:59, schrieb Ronald Weiss: >>> On 1. 4. 2014 22:23, Jens Lehmann wrote: Am 01.04.2014 01:35, schrieb Ronald Weiss: > On 1. 4. 2014 0:50, Ronald Weiss wrote: >> On 31. 3. 2014 23:47,

Re: [RFC] git submodule split

Am 02.04.2014 23:52, schrieb Michal Sojka: > Hello, > > I needed to convert a subdirectory of a repo to a submodule and have the > histories of both repos linked together. I found that this was discussed > few years back [1], but the code seemed quite complicated and was not > merged. > > [1]: >

Re: Makefile: Another "make" command is used when going into SUBDIR perl

Am 06.04.2014 01:10, schrieb Bjarni Ingi Gislason: Package: git-1.9.0 Another make command is used in the Makefile when it enters subdir PERL. The used "make" command is a link in my home directory to "/usr/sfw/bin/gmake". Other make commands are "/usr/ccs/bin/make" and "/usr/xpg4/bin/ma