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

2014-04-06 Thread René Scharfe

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/make.

My PATH variable has these directories in this order

$HOME, /usr/sfw/bin, /usr/xpg4/bin and /usr/ccs/bin

   The used variables for make are

CPPFLAGS=-I/usr/local/ssl/include
NO_GETTEXT=YesPlease
NO_TCLTK=YesPlease


Try adding MAKE = /usr/sfw/bin/gmake.

René

--
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


Re: [RFC] git submodule split

2014-04-06 Thread Michal Sojka
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 quite complicated and was not
 merged.
 
 [1]: 
 http://git.661346.n2.nabble.com/RFC-What-s-the-best-UI-for-git-submodule-split-tp2318127.html
 
 Now, the situation is better, because git subtree can already do most of
 the work. Below is a script that I used to split a submodule from my
 repo. It basically consist of a call to 'git subtree split' followed by
 'git filter-branch' to link the histories together.
 
 I'd like to get some initial feedback on it before attempting to
 integrate it with git sources (i.e. writing tests and doc). What do you
 think?

 Why do want to rewrite the whole history of the superproject,
 wouldn't it suffice to turn a directory into a submodule with
 the same content in a simple commit? 

I wanted to publish a project including its history but a part of that
project could not be made public due to legal reasons. Putting that part
into submodule seemed like best idea.

-Michal
--
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


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

2014-04-06 Thread Kirill Smelkov
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 k...@navytux.spb.ru writes:
 
  +extern
  +struct combine_diff_path *diff_tree_paths(
 
 These two on the same line, please.

Ok

  +   struct combine_diff_path *p, const unsigned char *sha1,
  +   const unsigned char **parent_sha1, int nparent,
  +   struct strbuf *base, struct diff_options *opt);
   extern int diff_tree_sha1(const unsigned char *old, const unsigned char 
  *new,
const char *base, struct diff_options *opt);
  ...
  +/*
  + * convert path - opt-diff_*() callbacks
  + *
  + * emits diff to first parent only, and tells diff tree-walker that we are 
  done
  + * with p and it can be freed.
  + */
  +static int emit_diff_first_parent_only(struct diff_options *opt, struct 
  combine_diff_path *p)
   {
 
 Very straight-forward; good.

Thanks

  +static struct combine_diff_path *path_appendnew(struct combine_diff_path 
  *last,
  +   int nparent, const struct strbuf *base, const char *path, int pathlen,
  +   unsigned mode, const unsigned char *sha1)
  +{
  +   struct combine_diff_path *p;
  +   int len = base-len + pathlen;
  +   int alloclen = combine_diff_path_size(nparent, len);
  +
  +   /* if last-next is !NULL - it is a pre-allocated memory, we can reuse 
  */
  +   p = last-next;
  +   if (p  (alloclen  (intptr_t)p-next)) {
  +   free(p);
  +   p = NULL;
  +   }
  +
  +   if (!p) {
  +   p = xmalloc(alloclen);
  +
  +   /*
  +* until we go to it next round, .next holds how many bytes we
  +* allocated (for faster realloc - we don't need copying old 
  data).
  +*/
  +   p-next = (struct combine_diff_path *)(intptr_t)alloclen;
 
 This reuse of the .next field is somewhat yucky, but it is very
 localized inside a function that has a single callsite to this
 function, so let's let it pass.

I agree it is not pretty, but it was the best approach I could find
for avoiding memory re-allocation without introducing new fields into
`struct combine_diff_path`. And yes, the trick is localized, so let's
let it live.


  +static struct combine_diff_path *emit_path(struct combine_diff_path *p,
  +   struct strbuf *base, struct diff_options *opt, int nparent,
  +   struct tree_desc *t, struct tree_desc *tp,
  +   int imin)
   {
 
 Again, fairly straight-forward and good.

Thanks again.


  +/*
  + * generate paths for combined diff D(sha1,parents_sha1[])
  + ...
  +static struct combine_diff_path *ll_diff_tree_paths(
  +   struct combine_diff_path *p, const unsigned char *sha1,
  +   const unsigned char **parents_sha1, int nparent,
  +   struct strbuf *base, struct diff_options *opt)
  +{
  +   struct tree_desc t, *tp;
  +   void *ttree, **tptree;
  +   int i;
  +
  +   tp = xalloca(nparent * sizeof(tp[0]));
  +   tptree = xalloca(nparent * sizeof(tptree[0]));
  +
  +   /*
  +* load parents first, as they are probably already cached.
  +*
  +* ( log_tree_diff() parses commit-parent before calling here via
  +*   diff_tree_sha1(parent, commit) )
  +*/
  +   for (i = 0; i  nparent; ++i)
  +   tptree[i] = fill_tree_descriptor(tp[i], parents_sha1[i]);
  +   ttree = fill_tree_descriptor(t, sha1);
   
  /* Enable recursion indefinitely */
  opt-pathspec.recursive = DIFF_OPT_TST(opt, RECURSIVE);
   
  for (;;) {
  -   int cmp;
  +   int imin, cmp;
   
  if (diff_can_quit_early(opt))
  break;
  +
  if (opt-pathspec.nr) {
  -   skip_uninteresting(t1, base, opt);
  -   skip_uninteresting(t2, base, opt);
  +   skip_uninteresting(t, base, opt);
  +   for (i = 0; i  nparent; i++)
  +   skip_uninteresting(tp[i], base, opt);
  }
  -   if (!t1.size  !t2.size)
  -   break;
   
  -   cmp = tree_entry_pathcmp(t1, t2);
  +   /* comparing is finished when all trees are done */
  +   if (!t.size) {
  +   int done = 1;
  +   for (i = 0; i  nparent; ++i)
  +   if (tp[i].size) {
  +   done = 0;
  +   break;
  +   }
  +   if (done)
  +   break;
  +   }
  +
  +   /*
  +* lookup imin = argmin(x1...xn),
  +* mark entries whether they =tp[imin] along the way
  +*/
  +   imin = 0;
  +   tp[0].entry.mode = ~S_IFXMIN_NEQ;
  +
  +   for (i = 1; i  nparent; ++i) {
  +   cmp = tree_entry_pathcmp(tp[i], tp[imin]);
  +   if (cmp  0) {
  +   imin = i;
  +   

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

2014-04-06 Thread Michael Haggerty
On 04/02/2014 06:58 PM, Junio C Hamano wrote:
 Jeff King p...@peff.net 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 in the face of hypothetical future programming errors.

 This also solves a race. We could be in the middle of rollback_lock_file
 when we get a signal, and double-close. It's probably not a big deal,
 though, since nobody could have opened a new descriptor in the interim
 that got the same number (so the second close will just fail silently).

 Still, this seems like a definite improvement.
 
 This is probably related to my comments on 2/22, but is fd the
 only thing that has a non-zero safe value?  Perhaps lock_file_init()
 that clears the structure fields to 0/NULL and fd to -1, and a
 convenience function lock_file_alloc() that does xmalloc() and then
 calls lock_file_init() may help us a bit when the lockfile structure
 is reused?

The first use of a lock_file object necessarily passes through
lock_file().  The only precondition for that function is that the
on_list field is zero, which is satisfied by a xcalloc()ed object.

Subsequent uses of a lock_file object must *not* zero the object.
lock_file objects are added to the lock_file_list and never removed.  So
zeroing a lock_file object would discard the rest of the linked list.
But subsequent uses must also pass through lock_file(), which sees that
on_list is set, and assumes that the object is in a self-consistent
state as left by commit_lock_file() or rollback_lock_file().

At least that's how it is supposed to work.  But lock_file objects are
in fact not cleaned up correctly in all circumstances.  The next version
of this patch series will work to fix that.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
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


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

2014-04-06 Thread Michael Haggerty
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 char 
 *path, int flags)
   */
  static const size_t max_path_len = sizeof(lk-filename) - 5;
  
 +if (!lock_file_list) {
 +/* One-time initialization */
 +sigchain_push_common(remove_lock_file_on_signal);
 +atexit(remove_lock_file);
 +}
 +
 +lk-owner = getpid();
 +if (!lk-on_list) {
 +/* Initialize *lk and add it to lock_file_list: */
 +lk-fd = -1;
 +lk-on_list = 1;
 +lk-filename[0] = 0;
 +lk-next = lock_file_list;
 +lock_file_list = lk;
 +}
 
 Initializing here is good, since we might be interrupted by a signal at
 any time. But what about during the locking procedure? We do:
 
 strcpy(lk-filename, path);
 if (!(flags  LOCK_NODEREF))
 resolve_symlink(lk-filename, max_path_len);
 strcat(lk-filename, .lock);
 
 So for a moment, lk-filename contains the name of the valuable file we
 are locking.  If we get a signal at that moment, do we accidentally
 delete it in remove_lock_file?
 
 I think the answer is no, because we check lk-owner before deleting,
 which will not match our pid (it should generally be zero due to xcalloc
 or static initialization, though perhaps we should clear it here).
 
 But that makes me wonder about the case of a reused lock. It will have
 lk-owner set from a previous invocation, and would potentially suffer
 from this problem. In other words, I think the change you are
 introducing does not have the problem, but the existing code does. :-/

Good point.  Yes, I agree that this is a problem in the existing code
and that it wasn't improved by my work.

 I didn't reproduce it experimentally, though.  We should be able to just
 
 lk-owner = 0;
 
 before the initial strcpy to fix it, I would think.

I think that using the owner field to avoid this problem is a bit
indirect, so I will soon submit a fix that involves adding a flag to
lock_file objects indicating whether the filename field currently
contains the name of a file that needs to be deleted.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
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


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

2014-04-06 Thread Michael Haggerty
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 = lk;
  lk-on_list = 1;
  }
 -if (adjust_shared_perm(lk-filename))
 -return error(cannot fix permission bits on %s,
 - lk-filename);
 +if (adjust_shared_perm(lk-filename)) {
 +error(cannot fix permission bits on %s, lk-filename);
 +rollback_lock_file(lk);
 +return -1;
 
 Would it make sense to change the order of rollback() and error()?
 Make the rollback first (and as early as possible) and whine then?

Thanks for the feedback.  It is a nice idea.  But given that
rollback_lock_file() erases the filename field, making the change you
suggest would require us to make a copy before calling
rollback_lock_file().  The only advantage would be that we hold the lock
file for a moment less, right?  Since this code path is only reached
when the repository has screwy permissions anyway, the next caller will
probably fail too.  So the extra code complication doesn't seem worth it
to me.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
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


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

2014-04-06 Thread Michael Haggerty
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 mhag...@alum.mit.edu
 ---
   config.c | 10 +-
   1 file changed, 5 insertions(+), 5 deletions(-)

 diff --git a/config.c b/config.c
 index 6821cef..1ea3f39 100644
 --- a/config.c
 +++ b/config.c
 @@ -1303,9 +1303,9 @@ static int store_aux(const char *key, const char
 *value, void *cb)
   return 0;
   }
   -static int write_error(const char *filename)
 +static int write_error(struct lock_file *lk)
 Does the write_error() really need to know about  struct lock_file ?
 (The name of the function does not indicate that it is doing something
 with lk)
 And if, would it make sense to rename it into
 write_error_and_do_something() ?

I'm going to leave this part out of the next re-roll, because you are
right: this change is mostly a distraction and probably not an improvement.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
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


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

2014-04-06 Thread Michael Haggerty
On 04/02/2014 07:19 PM, Junio C Hamano wrote:
 Michael Haggerty mhag...@alum.mit.edu 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 are planning to
 break the invariant lock_filename === filename + LOCK_SUFFIX in
 future changes in the series, it is understandable, though.

That is indeed the plan, but I'm splitting that change into a future
patch series because this one is getting big enough already dealing with
correctness issues.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
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


Segmentation fault on Mac OS Mavericks 10.9.2

2014-04-06 Thread davidhq
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 http://cl.ly/1h3F453F3A1i  
Crash report for 1.9.0 http://cl.ly/1x0N021L240V  

It works with 1.8.5.5

Sorry I'm not sure if this has already been reported... not 'capable' enough
to check and be sure this is the same problem.





--
View this message in context: 
http://git.661346.n2.nabble.com/Segmentation-fault-on-Mac-OS-Mavericks-10-9-2-tp7607349.html
Sent from the git mailing list archive at Nabble.com.
--
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


[PATCH] describe: rewrite name_rev() iteratively

2014-04-06 Thread Dragos Foianu
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 heap, but the
function no longer overflows the stack.

Reported-by: Sylvestre Ledru sylves...@mozilla.com
Signed-off-by: Dragos Foianu dragos.foi...@gmail.com
---
 builtin/name-rev.c |  176 ++--
 1 file changed, 128 insertions(+), 48 deletions(-)

diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index c824d4e..5848d81 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -19,66 +19,146 @@ static long cutoff = LONG_MAX;
 /* How many generations are maximally preferred over _one_ merge traversal? */
 #define MERGE_TRAVERSAL_WEIGHT 65535
 
+typedef struct rev_data {
+   struct commit *commit;
+   const char *tip_name;
+   int generation;
+   int distance;
+   int deref;
+} *rev_data;
+
+typedef struct rev_stack {
+   struct rev_data *rev;
+   struct rev_stack *next;
+} *rev_stack;
+
+static void stack_push(rev_stack *stack, rev_data data) {
+   rev_stack new_node = xmalloc(sizeof(*new_node));
+
+   new_node-rev = data;
+   new_node-next = *stack;
+   *stack = new_node;
+}
+
+static void stack_push_end(rev_stack *stack, rev_data data) {
+   rev_stack new_node = xmalloc(sizeof(*new_node));
+
+   while (*stack != NULL)
+   stack = (*stack)-next;
+   new_node-rev = data;
+   new_node-next = *stack;
+   *stack = new_node;
+}
+
+static rev_data stack_pop(rev_stack *stack) {
+   rev_stack next = (*stack)-next;
+   rev_data rev = (*stack)-rev;
+   free(*stack);
+
+   *stack = next;
+   return rev;
+}
+
+static rev_data make_rev_data(struct commit *commit,
+   const char* tip_name, int generation, int distance,
+   int deref)
+{
+   rev_data data = xmalloc(sizeof(*data));
+
+   data-commit = commit;
+   data-tip_name = tip_name;
+   data-generation = generation;
+   data-distance = distance;
+   data-deref = deref;
+
+   return data;
+}
+
 static void name_rev(struct commit *commit,
const char *tip_name, int generation, int distance,
int deref)
 {
-   struct rev_name *name = (struct rev_name *)commit-util;
-   struct commit_list *parents;
-   int parent_number = 1;
+   rev_stack stack = NULL;
+   rev_data data, next_rev;
 
-   parse_commit(commit);
+   data = make_rev_data(commit, tip_name, generation, distance, deref);
+   stack_push(stack, data);
 
-   if (commit-date  cutoff)
-   return;
+   while (stack != NULL) {
+   rev_data rev = stack_pop(stack);
 
-   if (deref) {
-   char *new_name = xmalloc(strlen(tip_name)+3);
-   strcpy(new_name, tip_name);
-   strcat(new_name, ^0);
-   tip_name = new_name;
+   struct rev_name *name = (struct rev_name *) rev-commit-util;
+   struct commit_list *parents;
+   int parent_number = 1;
 
-   if (generation)
-   die(generation: %d, but deref?, generation);
-   }
+   parse_commit(rev-commit);
+
+   if (rev-commit-date  cutoff)
+   continue;
+
+   if (rev-deref) {
+   char *new_name = xmalloc(strlen(rev-tip_name) + 3);
+   strcpy(new_name, rev-tip_name);
+   strcat(new_name, ^0);
+   rev-tip_name = new_name;
 
-   if (name == NULL) {
-   name = xmalloc(sizeof(rev_name));
-   commit-util = name;
-   goto copy_data;
-   } else if (name-distance  distance) {
+   if (rev-generation)
+   die(generation: %d, but deref?,
+   rev-generation);
+   }
+
+   if (name == NULL) {
+   name = xmalloc(sizeof(rev_name));
+   rev-commit-util = name;
+   goto copy_data;
+   } else if (name-distance  rev-distance) {
 copy_data:
-   name-tip_name = tip_name;
-   name-generation = generation;
-   name-distance = distance;
-   } else
-   return;
-
-   for (parents = commit-parents;
-   parents;
-   parents = parents-next, parent_number++) {
-   if (parent_number  1) {
-   int len = strlen(tip_name);
-   char *new_name = xmalloc(len +
-   1 + decimal_length(generation) +  /* ~n */
-   1 + 2 +   /* ^NN */
-

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

2014-04-06 Thread Philip Oakley


Junio C Hamano gits...@pobox.com 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 be formatted into a manual pages to begin with, and
also its contents are a bit stale.  It may be a good first step in
the right direction, but needs more work to at least get the
mark-up right before public consumption.

Will hold.



I do plan an update.

For some reason emails to me from git@vger.kernel.org have stopped being 
recieved (I was subscribed), so I'm trying to sort that.


Philip 



--
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


[PATCH v2 00/25] Lockfile correctness and refactoring

2014-04-06 Thread Michael Haggerty
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 retaining the lock.

But the review of v1 turned up even more correctness issues in the
existing implementation of lock files.  So this v2 dials back the
scope of the changes (it omits the new feature) but does more work to
fix problems with the current lock file implementation.

The main theme of this patch series is to better define the state
diagram for lock_file objects and to fix code that left them in
incorrect, indeterminate, or unexpected states.  There are also a few
patches that convert several functions to use strbufs instead of
limiting pathnames to a maximum length.

I hope that submitting these patches separately will make it easier
for them to be accepted without first having to decide wither the
activate-file-while-retaining-lock feature is a good one.

[1] http://thread.gmane.org/gmane.comp.version-control.git/245609

Michael Haggerty (25):
  api-lockfile: expand the documentation
  unable_to_lock_die(): rename function from unable_to_lock_index_die()
  rollback_lock_file(): do not clear filename redundantly
  rollback_lock_file(): set fd to -1
  lockfile: unlock file if lockfile permissions cannot be adjusted
  hold_lock_file_for_append(): release lock on errors
  lock_file(): always add lock_file object to lock_file_list
  struct lock_file: replace on_list field with flags field
  lockfile.c: document the various states of lock_file objects
  lockfile: define a constant LOCK_SUFFIX_LEN
  delete_ref_loose(): don't muck around in the lock_file's filename
  prepare_index(): declare return value to be (const char *)
  write_packed_entry_fn(): convert cb_data into a (const int *)
  lock_file(): exit early if lockfile cannot be opened
  remove_lock_file(): call rollback_lock_file()
  commit_lock_file(): inline temporary variable
  commit_lock_file(): make committing an unlocked lockfile a NOP
  lockfile: avoid transitory invalid states
  try_merge_strategy(): remove redundant lock_file allocation
  try_merge_strategy(): use a statically-allocated lock_file object
  commit_lock_file(): use a strbuf to manage temporary space
  Change lock_file::filename into a strbuf
  resolve_symlink(): use a strbuf for internal scratch space
  resolve_symlink(): take a strbuf parameter
  trim_last_path_elm(): replace last_path_elm()

 Documentation/technical/api-lockfile.txt |  40 -
 builtin/commit.c |  16 +-
 builtin/merge.c  |  15 +-
 builtin/reflog.c |   2 +-
 builtin/update-index.c   |   2 +-
 cache.h  |   6 +-
 config.c |   6 +-
 lockfile.c   | 282 +++
 refs.c   |  20 ++-
 shallow.c|   6 +-
 10 files changed, 243 insertions(+), 152 deletions(-)

-- 
1.9.1

--
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


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

2014-04-06 Thread Michael Haggerty
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.

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
Alternatively, we could make it a fatal error (e.g., an assert() or
if...die()) to call commit_lock_file() on an unlocked file, or omit a
warning in this case.  But since it is so hard to test code related to
locking failures, I have the feeling that such an error is most likely
to occur in some error-handling path, maybe when some other lockfile
acquisition has failed, and it would be better to let the code
continue its attempted cleanup instead of dying.  But it would be easy
to persuade me to change my opinion.

 Documentation/technical/api-lockfile.txt | 4 +++-
 lockfile.c   | 3 +++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/technical/api-lockfile.txt 
b/Documentation/technical/api-lockfile.txt
index b53e300..bfb2676 100644
--- a/Documentation/technical/api-lockfile.txt
+++ b/Documentation/technical/api-lockfile.txt
@@ -68,7 +68,9 @@ commit_lock_file::
with an earlier call to `hold_lock_file_for_update()`,
close the file descriptor and rename the lockfile to its
final destination.  Returns 0 upon success, a negative
-   value on failure to close(2) or rename(2).
+   value on failure to close(2) or rename(2).  It is a NOP to
+   call `commit_lock_file()` for a `lock_file` object that is not
+   currently locked.
 
 rollback_lock_file::
 
diff --git a/lockfile.c b/lockfile.c
index 47762c6..24aaa53 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -301,6 +301,9 @@ int commit_lock_file(struct lock_file *lk)
if (lk-fd = 0  close_lock_file(lk))
return -1;
 
+   if (!lk-filename[0])
+   return 0;
+
strcpy(result_file, lk-filename);
/* remove .lock: */
result_file[strlen(result_file) - LOCK_SUFFIX_LEN] = 0;
-- 
1.9.1

--
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


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

2014-04-06 Thread Michael Haggerty
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 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(struct lock_file *lk)
 {
char result_file[PATH_MAX];
-   size_t i;
+
if (lk-fd = 0  close_lock_file(lk))
return -1;
+
strcpy(result_file, lk-filename);
-   i = strlen(result_file) - LOCK_SUFFIX_LEN; /* .lock */
-   result_file[i] = 0;
+   /* remove .lock: */
+   result_file[strlen(result_file) - LOCK_SUFFIX_LEN] = 0;
+
if (rename(lk-filename, result_file))
return -1;
lk-filename[0] = 0;
-- 
1.9.1

--
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


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

2014-04-06 Thread Michael Haggerty
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 mhag...@alum.mit.edu
---
 builtin/merge.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index e15d0e1..f714961 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -671,7 +671,6 @@ static int try_merge_strategy(const char *strategy, struct 
commit_list *common,
if (!strcmp(strategy, recursive) || !strcmp(strategy, subtree)) {
int clean, x;
struct commit *result;
-   struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
int index_fd;
struct commit_list *reversed = NULL;
struct merge_options o;
-- 
1.9.1

--
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


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

2014-04-06 Thread Michael Haggerty
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 p...@peff.net
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 builtin/merge.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index f714961..0cde893 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -657,16 +657,16 @@ static int try_merge_strategy(const char *strategy, 
struct commit_list *common,
  struct commit_list *remoteheads,
  struct commit *head, const char *head_arg)
 {
+   static struct lock_file lock;
int index_fd;
-   struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
 
-   index_fd = hold_locked_index(lock, 1);
+   index_fd = hold_locked_index(lock, 1);
refresh_cache(REFRESH_QUIET);
if (active_cache_changed 
(write_cache(index_fd, active_cache, active_nr) ||
-commit_locked_index(lock)))
+commit_locked_index(lock)))
return error(_(Unable to write index.));
-   rollback_lock_file(lock);
+   rollback_lock_file(lock);
 
if (!strcmp(strategy, recursive) || !strcmp(strategy, subtree)) {
int clean, x;
@@ -699,14 +699,14 @@ static int try_merge_strategy(const char *strategy, 
struct commit_list *common,
for (j = common; j; j = j-next)
commit_list_insert(j-item, reversed);
 
-   index_fd = hold_locked_index(lock, 1);
+   index_fd = hold_locked_index(lock, 1);
clean = merge_recursive(o, head,
remoteheads-item, reversed, result);
if (active_cache_changed 
(write_cache(index_fd, active_cache, active_nr) 
||
-commit_locked_index(lock)))
+commit_locked_index(lock)))
die (_(unable to write %s), get_index_file());
-   rollback_lock_file(lock);
+   rollback_lock_file(lock);
return clean ? 0 : 1;
} else {
return try_merge_command(strategy, xopts_nr, xopts,
-- 
1.9.1

--
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


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

2014-04-06 Thread Michael Haggerty
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 effects, because lock_file objects
cannot be removed from the lock_file_list anyway.  But it is
unnecessary to leave this behavior inconsistent.

So change lock_file() to *always* ensure that the lock_file object is
registered in lock_file_list regardless of whether an error occurs.

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 lockfile.c | 25 -
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/lockfile.c b/lockfile.c
index e78a35f..120a6d6 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -130,6 +130,22 @@ static int lock_file(struct lock_file *lk, const char 
*path, int flags)
 */
static const size_t max_path_len = sizeof(lk-filename) - 5;
 
+   if (!lock_file_list) {
+   /* One-time initialization */
+   sigchain_push_common(remove_lock_file_on_signal);
+   atexit(remove_lock_file);
+   }
+
+   if (!lk-on_list) {
+   /* Initialize *lk and add it to lock_file_list: */
+   lk-fd = -1;
+   lk-owner = 0;
+   lk-on_list = 1;
+   lk-filename[0] = 0;
+   lk-next = lock_file_list;
+   lock_file_list = lk;
+   }
+
if (strlen(path) = max_path_len)
return -1;
strcpy(lk-filename, path);
@@ -138,16 +154,7 @@ static int lock_file(struct lock_file *lk, const char 
*path, int flags)
strcat(lk-filename, .lock);
lk-fd = open(lk-filename, O_RDWR | O_CREAT | O_EXCL, 0666);
if (0 = lk-fd) {
-   if (!lock_file_list) {
-   sigchain_push_common(remove_lock_file_on_signal);
-   atexit(remove_lock_file);
-   }
lk-owner = getpid();
-   if (!lk-on_list) {
-   lk-next = lock_file_list;
-   lock_file_list = lk;
-   lk-on_list = 1;
-   }
if (adjust_shared_perm(lk-filename)) {
error(cannot fix permission bits on %s, lk-filename);
rollback_lock_file(lk);
-- 
1.9.1

--
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


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

2014-04-06 Thread 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 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 recorded
by whether lk-filename was the empty string or 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 would have been 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 encoding part of the lock_file state in the filename
field, add a new bit LOCK_FLAGS_LOCKFILE_ACTIVE to flags, and use
this bit to distinguish between a lock_file object that is active
vs. one that is inactive.  Be careful to set this bit only when
filename really contains the name of a file that should be deleted on
cleanup.

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 lockfile.c | 55 +++
 1 file changed, 39 insertions(+), 16 deletions(-)

diff --git a/lockfile.c b/lockfile.c
index 24aaa53..b955cca 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -27,11 +27,14 @@
  * Instead of (3), the change can be rolled back by deleting lockfile.
  *
  * This module keeps track of all locked files in lock_file_list.
- * When the first file is locked, it registers an atexit(3) handler;
- * when the program exits, the handler rolls back any files that have
- * been locked but were never committed or rolled back.
+ * When the first file is locked, it registers an atexit(3) handler
+ * and a signal handler; when the program exits, the handler rolls
+ * back any files that have been locked but were never committed or
+ * rolled back.
  *
- * A lock_file object can be in several states:
+ * Because the signal handler can be called at any time, a lock_file
+ * object must always be in a well-defined state.  The possible states
+ * are as follows:
  *
  * - Uninitialized.  In this state the object's flags field must be
  *   zero but the rest of its contents need not be initialized.  As
@@ -40,18 +43,25 @@
  *   is set.
  *
  * - Locked, lockfile open (after hold_lock_file_for_update() or
- *   hold_lock_file_for_append()).  In this state, the lockfile
- *   exists, filename holds the filename of the lockfile, fd holds a
- *   file descriptor open for writing to the lockfile, and owner holds
- *   the PID of the process that locked the file.
+ *   hold_lock_file_for_append()).  In this state:
+ *   - the lockfile exists
+ *   - flags  LOCK_FLAGS_LOCKFILE_ACTIVE is set
+ *   - filename holds the filename of the lockfile
+ *   - fd holds a file descriptor open for writing to the lockfile
+ *   - owner holds the PID of the process that locked the file
  *
- * - Locked, lockfile closed (after close_lock_file()).  Same as the
- *   previous state, except that the lockfile is closed and fd is -1.
+ * - Locked, lockfile closed (after close_lock_file() or an
+ *   unsuccessful commit_lock_file()).  Same as the previous state,
+ *   except that the lockfile is closed and fd is -1.
  *
- * - Unlocked (after commit_lock_file(), rollback_lock_file(), or a
- *   failed attempt to lock).  In this state, filename[0] == '\0' and
- *   fd is -1.  The object is left registered in the lock_file_list,
- *   and flags  LOCK_FLAGS_ON_LIST is set.
+ * - Unlocked (after rollback_lock_file(), a successful
+ *   commit_lock_file(), or a failed attempt to lock).  In this state:
+ *   - flags  LOCK_FLAGS_LOCKFILE_ACTIVE is unset
+ *   - filename[0] == '\0' (usually, though there are transitory states
+ * in which this condition doesn't hold)
+ *   - fd is -1
+ *   - the object is left registered in the lock_file_list,
+ * and flags  LOCK_FLAGS_ON_LIST is set.
  *
  * See Documentation/api-lockfile.txt for more information.
  */
@@ -61,6 +71,13 @@
 /* This lock_file instance is in the lock_file_list */
 #define LOCK_FLAGS_ON_LIST 0x01
 
+/*
+ * The filename field points at an active lockfile (i.e., a file that
+ * needs to be deleted if the process dies)
+ */
+#define LOCK_FLAGS_LOCKFILE_ACTIVE 0x02
+
+
 static struct 

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

2014-04-06 Thread Michael Haggerty
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, unlocked).  Previously, the lockfile was
retained until process cleanup in this situation.

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 lockfile.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/lockfile.c b/lockfile.c
index 1122542..b101f77 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 = lk;
lk-on_list = 1;
}
-   if (adjust_shared_perm(lk-filename))
-   return error(cannot fix permission bits on %s,
-lk-filename);
+   if (adjust_shared_perm(lk-filename)) {
+   error(cannot fix permission bits on %s, lk-filename);
+   rollback_lock_file(lk);
+   return -1;
+   }
}
else
lk-filename[0] = 0;
-- 
1.9.1

--
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


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

2014-04-06 Thread Michael Haggerty
It is only necessary to clear the lock_file's filename field if it was
not already clear.

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 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
@@ -277,6 +277,6 @@ void rollback_lock_file(struct lock_file *lk)
if (lk-fd = 0)
close(lk-fd);
unlink_or_warn(lk-filename);
+   lk-filename[0] = 0;
}
-   lk-filename[0] = 0;
 }
-- 
1.9.1

--
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


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

2014-04-06 Thread Michael Haggerty
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 mhag...@alum.mit.edu
---
 lockfile.c | 53 +++--
 1 file changed, 19 insertions(+), 34 deletions(-)

diff --git a/lockfile.c b/lockfile.c
index c4e32a9..8e7bcda 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -133,53 +133,40 @@ static char *last_path_elm(char *p)
 #define MAXDEPTH 5
 
 /*
- * p = path that may be a symlink
- * s = full size of p
+ * path contains a path that might be a symlink.
  *
- * If p is a symlink, attempt to overwrite p with a path to the real
- * file or directory (which may or may not exist), following a chain of
- * symlinks if necessary.  Otherwise, leave p unmodified.
+ * If path is a symlink, attempt to overwrite it with a path to the
+ * real file or directory (which may or may not exist), following a
+ * chain of symlinks if necessary.  Otherwise, leave path unmodified.
  *
- * This is a best-effort routine.  If an error occurs, p will either be
- * left unmodified or will name a different symlink in a symlink chain
- * that started with p's initial contents.
- *
- * Always returns p.
+ * This is a best-effort routine.  If an error occurs, path will
+ * either be left unmodified or will name a different symlink in a
+ * symlink chain that started with the original path.
  */
-
-static char *resolve_symlink(char *p, size_t s)
+static void resolve_symlink(struct strbuf *path)
 {
int depth = MAXDEPTH;
static struct strbuf link = STRBUF_INIT;
 
while (depth--) {
-   if (strbuf_readlink(link, p, strlen(p))  0)
+   if (strbuf_readlink(link, path-buf, path-len)  0)
break;
 
-   if (is_absolute_path(link.buf)) {
+   if (is_absolute_path(link.buf))
/* absolute path simply replaces p */
-   if (link.len  s)
-   strcpy(p, link.buf);
-   else {
-   warning(%s: symlink too long, p);
-   break;
-   }
-   } else {
+   strbuf_reset(path);
+   else {
/*
 * link is a relative path, so replace the
 * last element of p with it.
 */
-   char *r = (char *)last_path_elm(p);
-   if (r - p + link.len  s)
-   strcpy(r, link.buf);
-   else {
-   warning(%s: symlink too long, p);
-   break;
-   }
+   char *r = last_path_elm(path-buf);
+   strbuf_setlen(path, r - path-buf);
}
+
+   strbuf_addbuf(path, link);
}
strbuf_reset(link);
-   return p;
 }
 
 /* We append .lock to the filename to derive the lockfile name: */
@@ -200,16 +187,14 @@ static int lock_file(struct lock_file *lk, const char 
*path, int flags)
lk-fd = -1;
lk-owner = 0;
lk-flags |= LOCK_FLAGS_ON_LIST;
-   strbuf_init(lk-filename, PATH_MAX);
+   strbuf_init(lk-filename, 0);
lk-next = lock_file_list;
lock_file_list = lk;
}
 
strbuf_addstr(lk-filename, path);
-   if (!(flags  LOCK_NODEREF)) {
-   resolve_symlink(lk-filename.buf, lk-filename.alloc);
-   strbuf_setlen(lk-filename, strlen(lk-filename.buf));
-   }
+   if (!(flags  LOCK_NODEREF))
+   resolve_symlink(lk-filename);
strbuf_addstr(lk-filename, .lock);
lk-fd = open(lk-filename.buf, O_RDWR | O_CREAT | O_EXCL, 0666);
if (lk-fd  0) {
-- 
1.9.1

--
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


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

2014-04-06 Thread Michael Haggerty
Aside from shortening and simplifying the code, this removes another
place where the path name length is arbitrarily limited.

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 lockfile.c | 33 -
 1 file changed, 12 insertions(+), 21 deletions(-)

diff --git a/lockfile.c b/lockfile.c
index 3befd7e..c4e32a9 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -150,44 +150,35 @@ static char *last_path_elm(char *p)
 static char *resolve_symlink(char *p, size_t s)
 {
int depth = MAXDEPTH;
+   static struct strbuf link = STRBUF_INIT;
 
while (depth--) {
-   char link[PATH_MAX];
-   int link_len = readlink(p, link, sizeof(link));
-   if (link_len  0) {
-   /* not a symlink anymore */
-   return p;
-   }
-   else if (link_len  sizeof(link))
-   /* readlink() never null-terminates */
-   link[link_len] = '\0';
-   else {
-   warning(%s: symlink too long, p);
-   return p;
-   }
+   if (strbuf_readlink(link, p, strlen(p))  0)
+   break;
 
-   if (is_absolute_path(link)) {
+   if (is_absolute_path(link.buf)) {
/* absolute path simply replaces p */
-   if (link_len  s)
-   strcpy(p, link);
+   if (link.len  s)
+   strcpy(p, link.buf);
else {
warning(%s: symlink too long, p);
-   return p;
+   break;
}
} else {
/*
-* link is a relative path, so I must replace the
+* link is a relative path, so replace the
 * last element of p with it.
 */
char *r = (char *)last_path_elm(p);
-   if (r - p + link_len  s)
-   strcpy(r, link);
+   if (r - p + link.len  s)
+   strcpy(r, link.buf);
else {
warning(%s: symlink too long, p);
-   return p;
+   break;
}
}
}
+   strbuf_reset(link);
return p;
 }
 
-- 
1.9.1

--
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


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

2014-04-06 Thread Michael Haggerty
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 mhag...@alum.mit.edu
---
 refs.c | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/refs.c b/refs.c
index 67fe8b7..64ea2f0 100644
--- a/refs.c
+++ b/refs.c
@@ -2485,12 +2485,14 @@ static int repack_without_ref(const char *refname)
 static int delete_ref_loose(struct ref_lock *lock, int flag)
 {
if (!(flag  REF_ISPACKED) || flag  REF_ISSYMREF) {
-   /* loose */
-   int err, i = strlen(lock-lk-filename) - 5; /* .lock */
-
-   lock-lk-filename[i] = 0;
-   err = unlink_or_warn(lock-lk-filename);
-   lock-lk-filename[i] = '.';
+   /*
+* loose.  The loose file name is the same as the
+* lockfile name, minus .lock:
+*/
+   char *loose_filename = xmemdupz(lock-lk-filename,
+   strlen(lock-lk-filename) - 5);
+   int err = unlink_or_warn(loose_filename);
+   free(loose_filename);
if (err  errno != ENOENT)
return 1;
}
-- 
1.9.1

--
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


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

2014-04-06 Thread Michael Haggerty
Avoid relying on the filename length restrictions that are currently
checked by lock_file().

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 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
+++ b/lockfile.c
@@ -316,7 +316,8 @@ int close_lock_file(struct lock_file *lk)
 
 int commit_lock_file(struct lock_file *lk)
 {
-   char result_file[PATH_MAX];
+   static struct strbuf result_file = STRBUF_INIT;
+   int err;
 
if (lk-fd = 0  close_lock_file(lk))
return -1;
@@ -324,11 +325,12 @@ int commit_lock_file(struct lock_file *lk)
if (!(lk-flags  LOCK_FLAGS_LOCKFILE_ACTIVE))
return 0;
 
-   strcpy(result_file, lk-filename);
/* remove .lock: */
-   result_file[strlen(result_file) - LOCK_SUFFIX_LEN] = 0;
-
-   if (rename(lk-filename, result_file))
+   strbuf_add(result_file, lk-filename,
+  strlen(lk-filename) - LOCK_SUFFIX_LEN);
+   err = rename(lk-filename, result_file.buf);
+   strbuf_reset(result_file);
+   if (err)
return -1;
lk-flags = ~LOCK_FLAGS_LOCKFILE_ACTIVE;
lk-filename[0] = 0;
-- 
1.9.1

--
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


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

2014-04-06 Thread Michael Haggerty
It does just what we need.

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 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)
pid_t me = getpid();
 
while (lock_file_list) {
-   if (lock_file_list-owner == me 
-   lock_file_list-filename[0]) {
-   if (lock_file_list-fd = 0)
-   close(lock_file_list-fd);
-   unlink_or_warn(lock_file_list-filename);
-   }
+   if (lock_file_list-owner == me)
+   rollback_lock_file(lock_file_list);
lock_file_list = lock_file_list-next;
}
 }
-- 
1.9.1

--
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


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

2014-04-06 Thread Michael Haggerty
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 mhag...@alum.mit.edu
---
 lockfile.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/lockfile.c b/lockfile.c
index 1bd0ae1..0879214 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -213,16 +213,16 @@ static int lock_file(struct lock_file *lk, const char 
*path, int flags)
resolve_symlink(lk-filename, max_path_len);
strcat(lk-filename, .lock);
lk-fd = open(lk-filename, O_RDWR | O_CREAT | O_EXCL, 0666);
-   if (0 = lk-fd) {
-   lk-owner = getpid();
-   if (adjust_shared_perm(lk-filename)) {
-   error(cannot fix permission bits on %s, lk-filename);
-   rollback_lock_file(lk);
-   return -1;
-   }
-   }
-   else
+   if (lk-fd  0) {
lk-filename[0] = 0;
+   return -1;
+   }
+   lk-owner = getpid();
+   if (adjust_shared_perm(lk-filename)) {
+   error(cannot fix permission bits on %s, lk-filename);
+   rollback_lock_file(lk);
+   return -1;
+   }
return lk-fd;
 }
 
-- 
1.9.1

--
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


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

2014-04-06 Thread Michael Haggerty
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 mhag...@alum.mit.edu
---
 builtin/commit.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index d9550c5..2e2d8bb 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -302,8 +302,8 @@ static void refresh_cache_or_die(int refresh_flags)
die_resolve_conflict(commit);
 }
 
-static char *prepare_index(int argc, const char **argv, const char *prefix,
-  const struct commit *current_head, int is_status)
+static const char *prepare_index(int argc, const char **argv, const char 
*prefix,
+const struct commit *current_head, int 
is_status)
 {
int fd;
struct string_list partial;
-- 
1.9.1

--
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


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

2014-04-06 Thread Michael Haggerty
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 mhag...@alum.mit.edu
---
 lockfile.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lockfile.c b/lockfile.c
index 7701267..1122542 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -275,7 +275,7 @@ void rollback_lock_file(struct lock_file *lk)
 {
if (lk-filename[0]) {
if (lk-fd = 0)
-   close(lk-fd);
+   close_lock_file(lk);
unlink_or_warn(lk-filename);
lk-filename[0] = 0;
}
-- 
1.9.1

--
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


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

2014-04-06 Thread Michael Haggerty
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 mhag...@alum.mit.edu
---
 Documentation/technical/api-lockfile.txt | 36 +---
 1 file changed, 33 insertions(+), 3 deletions(-)

diff --git a/Documentation/technical/api-lockfile.txt 
b/Documentation/technical/api-lockfile.txt
index dd89404..b53e300 100644
--- a/Documentation/technical/api-lockfile.txt
+++ b/Documentation/technical/api-lockfile.txt
@@ -28,9 +28,39 @@ hold_lock_file_for_update::
the final destination (e.g. `$GIT_DIR/index`) and a flag
`die_on_error`.  Attempt to create a lockfile for the
destination and return the file descriptor for writing
-   to the file.  If `die_on_error` flag is true, it dies if
-   a lock is already taken for the file; otherwise it
-   returns a negative integer to the caller on failure.
+   to the file.  The flags parameter is a combination of
++
+--
+LOCK_NODEREF::
+
+   Usually symbolic links in path are resolved in path and the
+   lockfile is created by adding .lock to the resolved path;
+   however, if `LOCK_NODEREF` is set, then the lockfile is
+   created by adding .lock to the path argument itself.
+
+LOCK_DIE_ON_ERROR::
+
+   If a lock is already taken for the file, `die()` with an error
+   message.  If this option is not specified, return a negative
+   integer to the caller on failure.
+--
+
+hold_lock_file_for_append::
+
+   Like `hold_lock_file_for_update()`, except that additionally
+   the existing contents of the file (if any) are copied to the
+   lockfile and its write pointer is positioned at the end of the
+   file before returning.
+
+unable_to_lock_error::
+
+   Emit an error describing that there was an error locking the
+   specified path.  The err parameter should be the errno of the
+   problem that caused the failure.
+
+unable_to_lock_die::
+
+   Like `unable_to_lock_error()`, but also `die()`.
 
 commit_lock_file::
 
-- 
1.9.1

--
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


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

2014-04-06 Thread Michael Haggerty
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 mhag...@alum.mit.edu
---
 lockfile.c | 52 
 1 file changed, 52 insertions(+)

diff --git a/lockfile.c b/lockfile.c
index 0486c58..2438430 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -4,6 +4,58 @@
 #include cache.h
 #include sigchain.h
 
+/*
+ * File write-locks as used by Git.
+ *
+ * When a file at $FILENAME needs to be written, it is done as
+ * follows:
+ *
+ * 1. Obtain a lock on the file by creating a lockfile at path
+ *$FILENAME.lock.  The file is opened for read/write using O_CREAT
+ *and O_EXCL mode to ensure that it doesn't already exist.  Such a
+ *lock file is respected by writers *but not by readers*.
+ *
+ *Usually, if $FILENAME is a symlink, then it is resolved, and the
+ *file ultimately pointed to is the one that is locked and later
+ *replaced.  However, if LOCK_NODEREF is used, then $FILENAME
+ *itself is locked and later replaced, even if it is a symlink.
+ *
+ * 2. Write the new file contents to the lockfile.
+ *
+ * 3. Move the lockfile to its final destination using rename(2).
+ *
+ * Instead of (3), the change can be rolled back by deleting lockfile.
+ *
+ * This module keeps track of all locked files in lock_file_list.
+ * When the first file is locked, it registers an atexit(3) handler;
+ * when the program exits, the handler rolls back any files that have
+ * been locked but were never committed or rolled back.
+ *
+ * A lock_file object can be in several states:
+ *
+ * - Uninitialized.  In this state the object's flags field must be
+ *   zero but the rest of its contents need not be initialized.  As
+ *   soon as the object is used in any way, it is irrevocably
+ *   registered in the lock_file_list, and flags  LOCK_FLAGS_ON_LIST
+ *   is set.
+ *
+ * - Locked, lockfile open (after hold_lock_file_for_update() or
+ *   hold_lock_file_for_append()).  In this state, the lockfile
+ *   exists, filename holds the filename of the lockfile, fd holds a
+ *   file descriptor open for writing to the lockfile, and owner holds
+ *   the PID of the process that locked the file.
+ *
+ * - Locked, lockfile closed (after close_lock_file()).  Same as the
+ *   previous state, except that the lockfile is closed and fd is -1.
+ *
+ * - Unlocked (after commit_lock_file(), rollback_lock_file(), or a
+ *   failed attempt to lock).  In this state, filename[0] == '\0' and
+ *   fd is -1.  The object is left registered in the lock_file_list,
+ *   and flags  LOCK_FLAGS_ON_LIST is set.
+ *
+ * See Documentation/api-lockfile.txt for more information.
+ */
+
 /* Values for lock_file::flags: */
 
 /* This lock_file instance is in the lock_file_list */
-- 
1.9.1

--
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


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

2014-04-06 Thread Michael Haggerty
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.  Rename the function accordingly.

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 lockfile.c | 38 --
 1 file changed, 16 insertions(+), 22 deletions(-)

diff --git a/lockfile.c b/lockfile.c
index 8e7bcda..f9c8515 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -100,32 +100,28 @@ static void remove_lock_file_on_signal(int signo)
 }
 
 /*
- * p = absolute or relative path name
+ * path = absolute or relative path name
  *
- * Return a pointer into p showing the beginning of the last path name
- * element.  If p is empty or the root directory (/), just return p.
+ * Remove the last path name element from path (leaving the preceding
+ * /, if any).  If path is empty or the root directory (/), set
+ * path to the empty string.
  */
-static char *last_path_elm(char *p)
+static void trim_last_path_elm(struct strbuf *path)
 {
-   /* r starts pointing to null at the end of the string */
-   char *r = strchr(p, '\0');
-
-   if (r == p)
-   return p; /* just return empty string */
-
-   r--; /* back up to last non-null character */
+   int i = path-len;
 
/* back up past trailing slashes, if any */
-   while (r  p  *r == '/')
-   r--;
+   while (i  path-buf[i - 1] == '/')
+   i--;
 
/*
-* then go backwards until I hit a slash, or the beginning of
-* the string
+* then go backwards until a slash, or the beginning of the
+* string
 */
-   while (r  p  *(r-1) != '/')
-   r--;
-   return r;
+   while (i  path-buf[i - 1] != '/')
+   i--;
+
+   strbuf_setlen(path, i);
 }
 
 
@@ -155,14 +151,12 @@ static void resolve_symlink(struct strbuf *path)
if (is_absolute_path(link.buf))
/* absolute path simply replaces p */
strbuf_reset(path);
-   else {
+   else
/*
 * link is a relative path, so replace the
 * last element of p with it.
 */
-   char *r = last_path_elm(path-buf);
-   strbuf_setlen(path, r - path-buf);
-   }
+   trim_last_path_elm(path);
 
strbuf_addbuf(path, link);
}
-- 
1.9.1

--
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


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

2014-04-06 Thread Michael Haggerty
This makes space for other bits, which will arrive soon.

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 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 @@ struct lock_file {
struct lock_file *next;
int fd;
pid_t owner;
-   char on_list;
+   unsigned char flags;
char filename[PATH_MAX];
 };
 #define LOCK_DIE_ON_ERROR 1
diff --git a/lockfile.c b/lockfile.c
index 120a6d6..0486c58 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -4,6 +4,11 @@
 #include cache.h
 #include sigchain.h
 
+/* Values for lock_file::flags: */
+
+/* This lock_file instance is in the lock_file_list */
+#define LOCK_FLAGS_ON_LIST 0x01
+
 static struct lock_file *lock_file_list;
 static const char *alternate_index_output;
 
@@ -136,11 +141,11 @@ static int lock_file(struct lock_file *lk, const char 
*path, int flags)
atexit(remove_lock_file);
}
 
-   if (!lk-on_list) {
+   if (!(lk-flags  LOCK_FLAGS_ON_LIST)) {
/* Initialize *lk and add it to lock_file_list: */
lk-fd = -1;
lk-owner = 0;
-   lk-on_list = 1;
+   lk-flags |= LOCK_FLAGS_ON_LIST;
lk-filename[0] = 0;
lk-next = lock_file_list;
lock_file_list = lk;
-- 
1.9.1

--
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


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

2014-04-06 Thread Michael Haggerty
This function is used for other things besides the index, so rename it
accordingly.

Suggested-by: Jeff King p...@peff.net
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 builtin/update-index.c | 2 +-
 cache.h| 2 +-
 lockfile.c | 6 +++---
 refs.c | 2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index d12ad95..3b1721b 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -891,7 +891,7 @@ int cmd_update_index(int argc, const char **argv, const 
char *prefix)
if (newfd  0) {
if (refresh_args.flags  REFRESH_QUIET)
exit(128);
-   unable_to_lock_index_die(get_index_file(), lock_error);
+   unable_to_lock_die(get_index_file(), lock_error);
}
if (write_cache(newfd, active_cache, active_nr) ||
commit_locked_index(lock_file))
diff --git a/cache.h b/cache.h
index 107ac61..3a873a4 100644
--- a/cache.h
+++ b/cache.h
@@ -543,7 +543,7 @@ struct lock_file {
 #define LOCK_DIE_ON_ERROR 1
 #define LOCK_NODEREF 2
 extern int unable_to_lock_error(const char *path, int err);
-extern NORETURN void unable_to_lock_index_die(const char *path, int err);
+extern NORETURN void unable_to_lock_die(const char *path, int err);
 extern int hold_lock_file_for_update(struct lock_file *, const char *path, 
int);
 extern int hold_lock_file_for_append(struct lock_file *, const char *path, 
int);
 extern int commit_lock_file(struct lock_file *);
diff --git a/lockfile.c b/lockfile.c
index 8fbcb6a..d4bfb3f 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -181,7 +181,7 @@ int unable_to_lock_error(const char *path, int err)
return -1;
 }
 
-NORETURN void unable_to_lock_index_die(const char *path, int err)
+NORETURN void unable_to_lock_die(const char *path, int err)
 {
die(%s, unable_to_lock_message(path, err));
 }
@@ -190,7 +190,7 @@ int hold_lock_file_for_update(struct lock_file *lk, const 
char *path, int flags)
 {
int fd = lock_file(lk, path, flags);
if (fd  0  (flags  LOCK_DIE_ON_ERROR))
-   unable_to_lock_index_die(path, errno);
+   unable_to_lock_die(path, errno);
return fd;
 }
 
@@ -201,7 +201,7 @@ int hold_lock_file_for_append(struct lock_file *lk, const 
char *path, int flags)
fd = lock_file(lk, path, flags);
if (fd  0) {
if (flags  LOCK_DIE_ON_ERROR)
-   unable_to_lock_index_die(path, errno);
+   unable_to_lock_die(path, errno);
return fd;
}
 
diff --git a/refs.c b/refs.c
index 28d5eca..67fe8b7 100644
--- a/refs.c
+++ b/refs.c
@@ -2118,7 +2118,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char 
*refname,
 */
goto retry;
else
-   unable_to_lock_index_die(ref_file, errno);
+   unable_to_lock_die(ref_file, errno);
}
return old_sha1 ? verify_lock(lock, old_sha1, mustexist) : lock;
 
-- 
1.9.1

--
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


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

2014-04-06 Thread Michael Haggerty
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 mhag...@alum.mit.edu
---
 lockfile.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lockfile.c b/lockfile.c
index b101f77..e78a35f 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -212,13 +212,13 @@ int hold_lock_file_for_append(struct lock_file *lk, const 
char *path, int flags)
if (errno != ENOENT) {
if (flags  LOCK_DIE_ON_ERROR)
die(cannot open '%s' for copying, path);
-   close(fd);
+   rollback_lock_file(lk);
return error(cannot open '%s' for copying, path);
}
} else if (copy_fd(orig_fd, fd)) {
if (flags  LOCK_DIE_ON_ERROR)
exit(128);
-   close(fd);
+   rollback_lock_file(lk);
return -1;
}
return fd;
-- 
1.9.1

--
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


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

2014-04-06 Thread Michael Haggerty
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.

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 builtin/commit.c | 12 ++--
 builtin/reflog.c |  2 +-
 cache.h  |  2 +-
 config.c |  6 +++---
 lockfile.c   | 45 +++--
 refs.c   |  6 +++---
 shallow.c|  6 +++---
 7 files changed, 36 insertions(+), 43 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 2e2d8bb..1833595 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -330,7 +330,7 @@ static const char *prepare_index(int argc, const char 
**argv, const char *prefix
die(_(unable to create temporary index));
 
old_index_env = getenv(INDEX_ENVIRONMENT);
-   setenv(INDEX_ENVIRONMENT, index_lock.filename, 1);
+   setenv(INDEX_ENVIRONMENT, index_lock.filename.buf, 1);
 
if (interactive_add(argc, argv, prefix, patch_interactive) != 0)
die(_(interactive add failed));
@@ -341,10 +341,10 @@ static const char *prepare_index(int argc, const char 
**argv, const char *prefix
unsetenv(INDEX_ENVIRONMENT);
 
discard_cache();
-   read_cache_from(index_lock.filename);
+   read_cache_from(index_lock.filename.buf);
 
commit_style = COMMIT_NORMAL;
-   return index_lock.filename;
+   return index_lock.filename.buf;
}
 
/*
@@ -368,7 +368,7 @@ static const char *prepare_index(int argc, const char 
**argv, const char *prefix
close_lock_file(index_lock))
die(_(unable to write new_index file));
commit_style = COMMIT_NORMAL;
-   return index_lock.filename;
+   return index_lock.filename.buf;
}
 
/*
@@ -453,9 +453,9 @@ static const char *prepare_index(int argc, const char 
**argv, const char *prefix
die(_(unable to write temporary index file));
 
discard_cache();
-   read_cache_from(false_lock.filename);
+   read_cache_from(false_lock.filename.buf);
 
-   return false_lock.filename;
+   return false_lock.filename.buf;
 }
 
 static int run_status(FILE *fp, const char *index_file, const char *prefix, 
int nowarn,
diff --git a/builtin/reflog.c b/builtin/reflog.c
index c12a9784..d7df34e 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -431,7 +431,7 @@ static int expire_reflog(const char *ref, const unsigned 
char *sha1, int unused,
 write_str_in_full(lock-lock_fd, \n) != 1 ||
 close_ref(lock)  0)) {
status |= error(Couldn't write %s,
-   lock-lk-filename);
+   lock-lk-filename.buf);
unlink(newlog_path);
} else if (rename(newlog_path, log_file)) {
status |= error(cannot rename %s to %s,
diff --git a/cache.h b/cache.h
index 59ce5cd..6ab2483 100644
--- a/cache.h
+++ b/cache.h
@@ -538,7 +538,7 @@ struct lock_file {
int fd;
pid_t owner;
unsigned char flags;
-   char filename[PATH_MAX];
+   struct strbuf filename;
 };
 #define LOCK_DIE_ON_ERROR 1
 #define LOCK_NODEREF 2
diff --git a/config.c b/config.c
index 6821cef..78c3dad 100644
--- a/config.c
+++ b/config.c
@@ -1706,7 +1706,7 @@ out_free:
return ret;
 
 write_err_out:
-   ret = write_error(lock-filename);
+   ret = write_error(lock-filename.buf);
goto out_free;
 
 }
@@ -1821,7 +1821,7 @@ int git_config_rename_section_in_file(const char 
*config_filename,
}
store.baselen = strlen(new_name);
if (!store_write_section(out_fd, new_name)) {
-   ret = write_error(lock-filename);
+   ret = write_error(lock-filename.buf);
goto out;
}
/*
@@ -1847,7 +1847,7 @@ int git_config_rename_section_in_file(const char 
*config_filename,
continue;
length = strlen(output);
if (write_in_full(out_fd, output, length) != length) {
-   ret = write_error(lock-filename);
+   ret = write_error(lock-filename.buf);

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

2014-04-06 Thread Michael Haggerty
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 mhag...@alum.mit.edu
---
 refs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/refs.c b/refs.c
index 64ea2f0..cd471ab 100644
--- a/refs.c
+++ b/refs.c
@@ -2176,7 +2176,7 @@ static void write_packed_entry(int fd, char *refname, 
unsigned char *sha1,
  */
 static int write_packed_entry_fn(struct ref_entry *entry, void *cb_data)
 {
-   int *fd = cb_data;
+   const int *fd = cb_data;
enum peel_status peel_status = peel_entry(entry, 0);
 
if (peel_status != PEEL_PEELED  peel_status != PEEL_NON_TAG)
-- 
1.9.1

--
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


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

2014-04-06 Thread Michael Haggerty
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 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)
return p;
 }
 
+/* We append .lock to the filename to derive the lockfile name: */
+#define LOCK_SUFFIX_LEN 5
 
 static int lock_file(struct lock_file *lk, const char *path, int flags)
 {
/*
-* subtract 5 from size to make sure there's room for adding
-* .lock for the lock file name
+* subtract LOCK_SUFFIX_LEN from size to make sure there's
+* room for adding .lock for the lock file name:
 */
-   static const size_t max_path_len = sizeof(lk-filename) - 5;
+   static const size_t max_path_len = sizeof(lk-filename) -
+  LOCK_SUFFIX_LEN;
 
if (!lock_file_list) {
/* One-time initialization */
@@ -302,7 +305,7 @@ int commit_lock_file(struct lock_file *lk)
if (lk-fd = 0  close_lock_file(lk))
return -1;
strcpy(result_file, lk-filename);
-   i = strlen(result_file) - 5; /* .lock */
+   i = strlen(result_file) - LOCK_SUFFIX_LEN; /* .lock */
result_file[i] = 0;
if (rename(lk-filename, result_file))
return -1;
-- 
1.9.1

--
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