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 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 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>
struct lock_file: declare some fields volatile

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 increase the chance that the signal handler will see a
valid object state.

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>
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 remove the
inner definition.

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 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 <peff@peff.net>
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 space

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>
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 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 <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
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.  Rename the function accordingly.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Makefile: use curl-config to determine curl flags

curl-config should always be installed alongside a curl distribution,
and its purpose is to provide flags for building against libcurl, so
use it instead of guessing flags and dependent libraries.

Allow overriding CURL_CONFIG to a custom path to curl-config, to
compile against a curl installation other than the first in PATH.

Depending on the set of features curl is compiled with, there may be
more libraries required than the previous two options of -lssl and
-lidn. For example, with a vanilla build of libcurl-7.36.0 on Mac OS X
10.9:

$ ~/d/curl-out-7.36.0/lib/curl-config --libs
-L/Users/dborowitz/d/curl-out-7.36.0/lib -lcurl -lgssapi_krb5 -lresolv -lldap -lz

Use this only when CURLDIR is not explicitly specified, to continue
supporting older builds.

Signed-off-by: Dave Borowitz <dborowitz@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Makefile: allow static linking against libcurl

This requires more flags than can be guessed with the old-style
CURLDIR and related options, so is only supported when curl-config is
present.

Signed-off-by: Dave Borowitz <dborowitz@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Merge branch 'mh/multimail' into jch

* mh/multimail:
  git-multimail: update to version 1.0.0
### match next
Merge branch 'cc/interpret-trailers' into jch

A new filter to programatically edit the tail end of the commit log
messages.

I was planning to merge it to 'next' and keep it there for the
remainder of the cycle, but it appears that there still will be
another round of reroll, at least for the documentation?

* cc/interpret-trailers:
  trailer: add blank line before the trailers if needed
  Documentation: add documentation for 'git interpret-trailers'
  trailer: add tests for commands in config file
  trailer: execute command from 'trailer.<name>.command'
  trailer: add tests for "git interpret-trailers"
  trailer: add interpret-trailers command
  trailer: put all the processing together and print
  trailer: parse trailers from stdin
  trailer: process command line trailer arguments
  trailer: read and process config information
  trailer: process trailers from stdin and arguments
  trailer: add data structures and basic functions
Merge branch 'nd/multiple-work-trees' into jch

A replacement for contrib/workdir/git-new-workdir that does not
rely on symbolic links and make sharing of objects and refs safer
by making the borrowee and borrowers aware of each other.

Will hold.

* nd/multiple-work-trees: (28 commits)
  count-objects: report unused files in $GIT_DIR/repos/...
  gc: support prune --repos
  gc: style change -- no SP before closing bracket
  prune: strategies for linked checkouts
  checkout: detach if the branch is already checked out elsewhere
  checkout: clean up half-prepared directories in --to mode
  checkout: support checking out into a new working directory
  use new wrapper write_file() for simple file writing
  wrapper.c: wrapper to open a file, fprintf then close
  setup.c: support multi-checkout repo setup
  setup.c: detect $GIT_COMMON_DIR check_repository_format_gently()
  setup.c: convert check_repository_format_gently to use strbuf
  setup.c: detect $GIT_COMMON_DIR in is_git_directory()
  setup.c: convert is_git_directory() to use strbuf
  git-stash: avoid hardcoding $GIT_DIR/logs/....
  *.sh: avoid hardcoding $GIT_DIR/hooks/...
  git-sh-setup.sh: use rev-parse --git-path to get $GIT_DIR/objects
  $GIT_COMMON_DIR: a new environment variable
  commit: use SEQ_DIR instead of hardcoding "sequencer"
  fast-import: use git_path() for accessing .git dir instead of get_git_dir()
  ...

# Conflicts:
# 	builtin/repack.c
Merge branch 'mh/ref-transaction' into jch

Update "update-ref --stdin [-z]" and then introduce a transactional
support for (multi-)reference updates.

* mh/ref-transaction: (27 commits)
  ref_transaction_commit(): work with transaction->updates in place
  struct ref_update: add a type field
  struct ref_update: add a lock field
  ref_transaction_commit(): simplify code using temporary variables
  struct ref_update: store refname as a FLEX_ARRAY
  struct ref_update: rename field "ref_name" to "refname"
  refs: remove API function update_refs()
  update-ref --stdin: reimplement using reference transactions
  refs: add a concept of a reference transaction
  update-ref --stdin: harmonize error messages
  update-ref --stdin: improve the error message for unexpected EOF
  t1400: test one mistake at a time
  update-ref --stdin -z: deprecate interpreting the empty string as zeros
  update-ref.c: extract a new function, parse_next_sha1()
  t1400: test that stdin -z update treats empty <newvalue> as zeros
  update-ref --stdin: simplify error messages for missing oldvalues
  update-ref --stdin: make error messages more consistent
  update-ref --stdin: improve error messages for invalid values
  update-ref.c: extract a new function, parse_refname()
  parse_cmd_verify(): copy old_sha1 instead of evaluating <oldvalue> twice
  ...

# Conflicts:
# 	builtin/update-ref.c
# 	refs.h
Merge branch 'jl/status-added-submodule-is-never-ignored' into jch

There also are a few patches Ronald Weiss and Jens are working on
polishing around this topic, and a patch from Jens each for gitk
and git-gui.

* jl/status-added-submodule-is-never-ignored:
  commit -m: commit staged submodules regardless of ignore config
  status/commit: show staged submodules regardless of ignore config
Merge branch 'mh/lockfile' into jch

Refactor and fix corner-case bugs in the lockfile API, all looked
sensible.

Still being commented on.

* mh/lockfile: (25 commits)
  trim_last_path_elm(): replace last_path_elm()
  resolve_symlink(): take a strbuf parameter
  resolve_symlink(): use a strbuf for internal scratch space
  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
  commit_lock_file(): die() if called for unlocked lockfile object
  commit_lock_file(): inline temporary variable
  remove_lock_file(): call rollback_lock_file()
  lock_file(): exit early if lockfile cannot be opened
  write_packed_entry_fn(): convert cb_data into a (const int *)
  prepare_index(): declare return value to be (const char *)
  delete_ref_loose(): don't muck around in the lock_file's filename
  cache.h: define constants LOCK_SUFFIX and LOCK_SUFFIX_LEN
  lockfile.c: document the various states of lock_file objects
  lock_file(): always add lock_file object to lock_file_list
  hold_lock_file_for_append(): release lock on errors
  ...
Merge branch 'fc/remote-helper-fixes' into jch

* fc/remote-helper-fixes:
  remote-bzr: trivial test fix
  remote-bzr: include authors field in pushed commits
  remote-bzr: add support for older versions
  remote-hg: always normalize paths
  remote-helpers: allow all tests running from any dir
Merge branch 'fc/complete-aliased-push' into jch

* fc/complete-aliased-push:
  completion: fix completing args of aliased "push", "fetch", etc.