Commit Graph

74 Commits

Author SHA1 Message Date
Jeff King
0e5bba53af add UNLEAK annotation for reducing leak false positives
It's a common pattern in git commands to allocate some
memory that should last for the lifetime of the program and
then not bother to free it, relying on the OS to throw it
away.

This keeps the code simple, and it's fast (we don't waste
time traversing structures or calling free at the end of the
program). But it also triggers warnings from memory-leak
checkers like valgrind or LSAN. They know that the memory
was still allocated at program exit, but they don't know
_when_ the leaked memory stopped being useful. If it was
early in the program, then it's probably a real and
important leak. But if it was used right up until program
exit, it's not an interesting leak and we'd like to suppress
it so that we can see the real leaks.

This patch introduces an UNLEAK() macro that lets us do so.
To understand its design, let's first look at some of the
alternatives.

Unfortunately the suppression systems offered by
leak-checking tools don't quite do what we want. A
leak-checker basically knows two things:

  1. Which blocks were allocated via malloc, and the
     callstack during the allocation.

  2. Which blocks were left un-freed at the end of the
     program (and which are unreachable, but more on that
     later).

Their suppressions work by mentioning the function or
callstack of a particular allocation, and marking it as OK
to leak.  So imagine you have code like this:

  int cmd_foo(...)
  {
	/* this allocates some memory */
	char *p = some_function();
	printf("%s", p);
	return 0;
  }

You can say "ignore allocations from some_function(),
they're not leaks". But that's not right. That function may
be called elsewhere, too, and we would potentially want to
know about those leaks.

So you can say "ignore the callstack when main calls
some_function".  That works, but your annotations are
brittle. In this case it's only two functions, but you can
imagine that the actual allocation is much deeper. If any of
the intermediate code changes, you have to update the
suppression.

What we _really_ want to say is that "the value assigned to
p at the end of the function is not a real leak". But
leak-checkers can't understand that; they don't know about
"p" in the first place.

However, we can do something a little bit tricky if we make
some assumptions about how leak-checkers work. They
generally don't just report all un-freed blocks. That would
report even globals which are still accessible when the
leak-check is run.  Instead they take some set of memory
(like BSS) as a root and mark it as "reachable". Then they
scan the reachable blocks for anything that looks like a
pointer to a malloc'd block, and consider that block
reachable. And then they scan those blocks, and so on,
transitively marking anything reachable from a global as
"not leaked" (or at least leaked in a different category).

So we can mark the value of "p" as reachable by putting it
into a variable with program lifetime. One way to do that is
to just mark "p" as static. But that actually affects the
run-time behavior if the function is called twice (you
aren't likely to call main() twice, but some of our cmd_*()
functions are called from other commands).

Instead, we can trick the leak-checker by putting the value
into _any_ reachable bytes. This patch keeps a global
linked-list of bytes copied from "unleaked" variables. That
list is reachable even at program exit, which confers
recursive reachability on whatever values we unleak.

In other words, you can do:

  int cmd_foo(...)
  {
	char *p = some_function();
	printf("%s", p);
	UNLEAK(p);
	return 0;
  }

to annotate "p" and suppress the leak report.

But wait, couldn't we just say "free(p)"? In this toy
example, yes. But UNLEAK()'s byte-copying strategy has
several advantages over actually freeing the memory:

  1. It's recursive across structures. In many cases our "p"
     is not just a pointer, but a complex struct whose
     fields may have been allocated by a sub-function. And
     in some cases (e.g., dir_struct) we don't even have a
     function which knows how to free all of the struct
     members.

     By marking the struct itself as reachable, that confers
     reachability on any pointers it contains (including those
     found in embedded structs, or reachable by walking
     heap blocks recursively.

  2. It works on cases where we're not sure if the value is
     allocated or not. For example:

       char *p = argc > 1 ? argv[1] : some_function();

     It's safe to use UNLEAK(p) here, because it's not
     freeing any memory. In the case that we're pointing to
     argv here, the reachability checker will just ignore
     our bytes.

  3. Likewise, it works even if the variable has _already_
     been freed. We're just copying the pointer bytes. If
     the block has been freed, the leak-checker will skip
     over those bytes as uninteresting.

  4. Because it's not actually freeing memory, you can
     UNLEAK() before we are finished accessing the variable.
     This is helpful in cases like this:

       char *p = some_function();
       return another_function(p);

     Writing this with free() requires:

       int ret;
       char *p = some_function();
       ret = another_function(p);
       free(p);
       return ret;

     But with unleak we can just write:

       char *p = some_function();
       UNLEAK(p);
       return another_function(p);

This patch adds the UNLEAK() macro and enables it
automatically when Git is compiled with SANITIZE=leak.  In
normal builds it's a noop, so we pay no runtime cost.

It also adds some UNLEAK() annotations to show off how the
feature works. On top of other recent leak fixes, these are
enough to get t0000 and t0001 to pass when compiled with
LSAN.

Note the case in commit.c which actually converts a
strbuf_release() into an UNLEAK. This code was already
non-leaky, but the free didn't do anything useful, since
we're exiting. Converting it to an annotation means that
non-leak-checking builds pay no runtime cost. The cost is
minimal enough that it's probably not worth going on a
crusade to convert these kinds of frees to UNLEAKS. I did it
here for consistency with the "sb" leak (though it would
have been equally correct to go the other way, and turn them
both into strbuf_release() calls).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-09-08 15:43:17 +09:00
Junio C Hamano
50f03c6676 Merge branch 'ab/free-and-null'
A common pattern to free a piece of memory and assign NULL to the
pointer that used to point at it has been replaced with a new
FREE_AND_NULL() macro.

* ab/free-and-null:
  *.[ch] refactoring: make use of the FREE_AND_NULL() macro
  coccinelle: make use of the "expression" FREE_AND_NULL() rule
  coccinelle: add a rule to make "expression" code use FREE_AND_NULL()
  coccinelle: make use of the "type" FREE_AND_NULL() rule
  coccinelle: add a rule to make "type" code use FREE_AND_NULL()
  git-compat-util: add a FREE_AND_NULL() wrapper around free(ptr); ptr = NULL
2017-06-24 14:28:41 -07:00
Junio C Hamano
f31d23a399 Merge branch 'bw/config-h'
Fix configuration codepath to pay proper attention to commondir
that is used in multi-worktree situation, and isolate config API
into its own header file.

* bw/config-h:
  config: don't implicitly use gitdir or commondir
  config: respect commondir
  setup: teach discover_git_directory to respect the commondir
  config: don't include config.h by default
  config: remove git_config_iter
  config: create config.h
2017-06-24 14:28:41 -07:00
Ævar Arnfjörð Bjarmason
88ce3ef636 *.[ch] refactoring: make use of the FREE_AND_NULL() macro
Replace occurrences of `free(ptr); ptr = NULL` which weren't caught by
the coccinelle rule. These fall into two categories:

 - free/NULL assignments one after the other which coccinelle all put
   on one line, which is functionally equivalent code, but very ugly.

 - manually spotted occurrences where the NULL assignment isn't right
   after the free() call.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-16 12:44:09 -07:00
Brandon Williams
b2141fc1d2 config: don't include config.h by default
Stop including config.h by default in cache.h.  Instead only include
config.h in those files which require use of the config system.

Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-15 12:56:22 -07:00
Junio C Hamano
849e671b52 Merge branch 'js/plug-leaks'
Fix memory leaks pointed out by Coverity (and people).

* js/plug-leaks: (26 commits)
  checkout: fix memory leak
  submodule_uses_worktrees(): plug memory leak
  show_worktree(): plug memory leak
  name-rev: avoid leaking memory in the `deref` case
  remote: plug memory leak in match_explicit()
  add_reflog_for_walk: avoid memory leak
  shallow: avoid memory leak
  line-log: avoid memory leak
  receive-pack: plug memory leak in update()
  fast-export: avoid leaking memory in handle_tag()
  mktree: plug memory leaks reported by Coverity
  pack-redundant: plug memory leak
  setup_discovered_git_dir(): plug memory leak
  setup_bare_git_dir(): help static analysis
  split_commit_in_progress(): simplify & fix memory leak
  checkout: fix memory leak
  cat-file: fix memory leak
  mailinfo & mailsplit: check for EOF while parsing
  status: close file descriptor after reading git-rebase-todo
  difftool: address a couple of resource/memory leaks
  ...
2017-05-29 12:34:44 +09:00
Junio C Hamano
b15667bbdc Merge branch 'js/larger-timestamps'
Some platforms have ulong that is smaller than time_t, and our
historical use of ulong for timestamp would mean they cannot
represent some timestamp that the platform allows.  Invent a
separate and dedicated timestamp_t (so that we can distingiuish
timestamps and a vanilla ulongs, which along is already a good
move), and then declare uintmax_t is the type to be used as the
timestamp_t.

* js/larger-timestamps:
  archive-tar: fix a sparse 'constant too large' warning
  use uintmax_t for timestamps
  date.c: abort if the system time cannot handle one of our timestamps
  timestamp_t: a new data type for timestamps
  PRItime: introduce a new "printf format" for timestamps
  parse_timestamp(): specify explicitly where we parse timestamps
  t0006 & t5000: skip "far in the future" test when time_t is too limited
  t0006 & t5000: prepare for 64-bit timestamps
  ref-filter: avoid using `unsigned long` for catch-all data type
2017-05-16 11:51:59 +09:00
Johannes Schindelin
2e11f58fa6 show_worktree(): plug memory leak
The buffer allocated by shorten_unambiguous_ref() needs to be released.

Discovered by Coverity.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-05-08 12:18:20 +09:00
Johannes Schindelin
dddbad728c timestamp_t: a new data type for timestamps
Git's source code assumes that unsigned long is at least as precise as
time_t. Which is incorrect, and causes a lot of problems, in particular
where unsigned long is only 32-bit (notably on Windows, even in 64-bit
versions).

So let's just use a more appropriate data type instead. In preparation
for this, we introduce the new `timestamp_t` data type.

By necessity, this is a very, very large patch, as it has to replace all
timestamps' data type in one go.

As we will use a data type that is not necessarily identical to `time_t`,
we need to be very careful to use `time_t` whenever we interact with the
system functions, and `timestamp_t` everywhere else.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-04-27 13:07:39 +09:00
Junio C Hamano
e31159746e Merge branch 'nd/worktree-add-lock'
Allow to lock a worktree immediately after it's created. This helps
prevent a race between "git worktree add; git worktree lock" and
"git worktree prune".

* nd/worktree-add-lock:
  worktree add: add --lock option
2017-04-26 15:39:12 +09:00
Junio C Hamano
b80f629f5b Merge branch 'jk/war-on-git-path'
While handy, "git_path()" is a dangerous function to use as a
callsite that uses it safely one day can be broken by changes
to other code that calls it.  Reduction of its use continues.

* jk/war-on-git-path:
  am: drop "dir" parameter from am_state_init
  replace strbuf_addstr(git_path()) with git_path_buf()
  replace xstrdup(git_path(...)) with git_pathdup(...)
  use git_path_* helper functions
  branch: add edit_description() helper
  bisect: add git_path_bisect_terms helper
2017-04-26 15:39:08 +09:00
Jeff King
8c2ca3a6d6 replace strbuf_addstr(git_path()) with git_path_buf()
Writing directly into the strbuf avoids a useless copy of
the data, and dropping calls to git_path() makes it easier
to audit for dangerous calls.

Note that git_path() does an implicit strbuf_reset(), but in
each of these cases we were either already doing that reset,
or writing into a fresh strbuf anyway.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-04-20 21:04:20 -07:00
Nguyễn Thái Ngọc Duy
507e6e9eec worktree add: add --lock option
As explained in the document. This option has an advantage over the
command sequence "git worktree add && git worktree lock": there will be
no gap that somebody can accidentally "prune" the new worktree (or soon,
explicitly "worktree remove" it).

"worktree add" does keep a lock on while it's preparing the worktree.
If --lock is specified, this lock remains after the worktree is created.

Suggested-by: David Taylor <David.Taylor@dell.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-04-20 17:59:02 -07:00
Jeff King
e4da43b1f0 prefix_filename: return newly allocated string
The prefix_filename() function returns a pointer to static
storage, which makes it easy to use dangerously. We already
fixed one buggy caller in hash-object recently, and the
calls in apply.c are suspicious (I didn't dig in enough to
confirm that there is a bug, but we call the function once
in apply_all_patches() and then again indirectly from
parse_chunk()).

Let's make it harder to get wrong by allocating the return
value. For simplicity, we'll do this even when the prefix is
empty (and we could just return the original file pointer).
That will cause us to allocate sometimes when we wouldn't
otherwise need to, but this function isn't called in
performance critical code-paths (and it already _might_
allocate on any given call, so a caller that cares about
performance is questionable anyway).

The downside is that the callers need to remember to free()
the result to avoid leaking. Most of them already used
xstrdup() on the result, so we know they are OK. The
remainder have been converted to use free() as appropriate.

I considered retaining a prefix_filename_unsafe() for cases
where we know the static lifetime is OK (and handling the
cleanup is awkward). This is only a handful of cases,
though, and it's not worth the mental energy in worrying
about whether the "unsafe" variant is OK to use in any
situation.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-21 11:18:41 -07:00
Jeff King
116fb64e43 prefix_filename: drop length parameter
This function takes the prefix as a ptr/len pair, but in
every caller the length is exactly strlen(ptr). Let's
simplify the interface and just take the string. This saves
callers specifying it (and in some cases handling a NULL
prefix).

In a handful of cases we had the length already without
calling strlen, so this is technically slower. But it's not
likely to matter (after all, if the prefix is non-empty
we'll allocate and copy it into a buffer anyway).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-21 11:12:53 -07:00
Junio C Hamano
65fecf0c08 Merge branch 'ps/worktree-prune-help-fix'
Incorrect usage help message for "git worktree prune" has been fixed.

* ps/worktree-prune-help-fix:
  worktree: fix option descriptions for `prune`
2017-02-10 12:52:25 -08:00
Patrick Steinhardt
2488dcab22 worktree: fix option descriptions for prune
The `verbose` and `expire` options of the `git worktree prune`
subcommand have wrong descriptions in that they pretend to relate to
objects. But as the git-worktree(1) correctly states, these options have
nothing to do with objects but only with worktrees. Fix the description
accordingly.

Signed-off-by: Patrick Steinhardt <patrick.steinhardt@elego.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-02-06 10:59:25 -08:00
Nguyễn Thái Ngọc Duy
4df1d4d466 worktree list: keep the list sorted
It makes it easier to write tests for. But it should also be good for
the user since locating a worktree by eye would be easier once they
notice this.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-11-28 13:18:51 -08:00
Nguyễn Thái Ngọc Duy
4fff1ef7ff worktree.c: get_worktrees() takes a new flag argument
This is another no-op patch, in preparation for get_worktrees() to do
optional things, like sorting.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-11-28 13:18:51 -08:00
Nguyễn Thái Ngọc Duy
a234563a3b get_worktrees() must return main worktree as first item even on error
This is required by git-worktree.txt, stating that the main worktree is
the first line (especially in --porcelain mode when we can't just change
behavior at will).

There's only one case when get_worktrees() may skip main worktree, when
parse_ref() fails. Update the code so that we keep first item as main
worktree and return something sensible in this case:

 - In user-friendly mode, since we're not constraint by anything,
   returning "(error)" should do the job (we already show "(detached
   HEAD)" which is not machine-friendly). Actually errors should be
   printed on stderr by parse_ref() (*)

 - In plumbing mode, we do not show neither 'bare', 'detached' or
   'branch ...', which is possible by the format description if I read
   it right.

Careful readers may realize that when the local variable "head_ref" in
get_main_worktree() is emptied, add_head_info() will do nothing to
wt->head_sha1. But that's ok because head_sha1 is zero-ized in the
previous patch.

(*) Well, it does not. But it's supposed to be a stop gap implementation
    until we can reuse refs code to parse "ref: " stuff in HEAD, from
    resolve_refs_unsafe(). Now may be the time since refs refactoring is
    mostly done.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-11-28 13:18:51 -08:00
Nguyễn Thái Ngọc Duy
96f09e2a11 worktree: reorder an if statement
This is no-op. But it helps reduce diff noise in the next patch.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-11-28 13:18:51 -08:00
Junio C Hamano
d49028e6e7 worktree: honor configuration variables
The command accesses default_abbrev (defined in environment.c and is
updated via core.abbrev configuration), but never makes any call to
git_config().  The output from "worktree list" ignores the abbrev
setting for this reason.

Make a call to git_config() to read the default set of configuration
variables at the beginning of the command.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-27 10:51:33 -07:00
René Scharfe
542aa25d97 use CHILD_PROCESS_INIT to initialize automatic variables
Initialize struct child_process variables already when they're defined.
That's shorter and saves a function call.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-08-05 15:10:05 -07:00
Junio C Hamano
2c608e0f7c Merge branch 'nd/worktree-lock'
"git worktree prune" protected worktrees that are marked as
"locked" by creating a file in a known location.  "git worktree"
command learned a dedicated command pair to create and remove such
a file, so that the users do not have to do this with editor.

* nd/worktree-lock:
  worktree.c: find_worktree() search by path suffix
  worktree: add "unlock" command
  worktree: add "lock" command
  worktree.c: add is_worktree_locked()
  worktree.c: add is_main_worktree()
  worktree.c: add find_worktree()
2016-07-28 10:34:42 -07:00
Nguyễn Thái Ngọc Duy
6d308627ca worktree: add "unlock" command
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-07-08 15:31:04 -07:00
Nguyễn Thái Ngọc Duy
58142c09a4 worktree: add "lock" command
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-07-08 15:31:04 -07:00
Jeff King
dabd35f4cd avoid using sha1_to_hex output as printf format
We know that it should not contain any percent-signs, but
it's a good habit not to feed non-literals to printf
formatters.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-07-08 10:11:27 -07:00
Junio C Hamano
7a738b40f6 Merge branch 'nd/worktree-cleanup-post-head-protection'
Further preparatory clean-up for "worktree" feature continues.

* nd/worktree-cleanup-post-head-protection:
  worktree: simplify prefixing paths
  worktree: avoid 0{40}, too many zeroes, hard to read
  worktree.c: use is_dot_or_dotdot()
  git-worktree.txt: keep subcommand listing in alphabetical order
  worktree.c: rewrite mark_current_worktree() to avoid strbuf
  completion: support git-worktree
2016-07-06 13:38:11 -07:00
Jordan DE GEA
1a450e2fd1 worktree: allow "-" short-hand for @{-1} in add command
Since `git worktree add` uses `git checkout` when `[<branch>]` is used,
and `git checkout -` is already supported, it makes sense to allow the
same shortcut in `git worktree add`.

Signed-off-by: Jordan DE GEA <jordan.de-gea@grenoble-inp.org>
Signed-off-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-05-31 12:28:25 -07:00
Nguyễn Thái Ngọc Duy
0409e0b6dc worktree: simplify prefixing paths
This also makes slash conversion always happen on Windows (a side effect
of prefix_filename). Which is a good thing.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Reviewed-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-05-24 13:19:23 -07:00
Nguyễn Thái Ngọc Duy
ef23c347cf worktree: avoid 0{40}, too many zeroes, hard to read
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Reviewed-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-05-24 13:19:22 -07:00
Nguyễn Thái Ngọc Duy
afb9e30b2c worktree.c: use is_dot_or_dotdot()
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Reviewed-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-05-24 13:19:22 -07:00
Nguyễn Thái Ngọc Duy
7b722d906b git-worktree.txt: keep subcommand listing in alphabetical order
This is probably not the best order. But it makes it no-brainer to know
where to insert new commands. At some point we might want to reorder at
least the synopsis part again, grouping commonly use subcommands together.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Reviewed-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-05-24 13:19:22 -07:00
Junio C Hamano
352d72a30e Merge branch 'nd/worktree-various-heads'
The experimental "multiple worktree" feature gains more safety to
forbid operations on a branch that is checked out or being actively
worked on elsewhere, by noticing that e.g. it is being rebased.

* nd/worktree-various-heads:
  branch: do not rename a branch under bisect or rebase
  worktree.c: check whether branch is bisected in another worktree
  wt-status.c: split bisect detection out of wt_status_get_state()
  worktree.c: check whether branch is rebased in another worktree
  worktree.c: avoid referencing to worktrees[i] multiple times
  wt-status.c: make wt_status_check_rebase() work on any worktree
  wt-status.c: split rebase detection out of wt_status_get_state()
  path.c: refactor and add worktree_git_path()
  worktree.c: mark current worktree
  worktree.c: make find_shared_symref() return struct worktree *
  worktree.c: store "id" instead of "git_dir"
  path.c: add git_common_path() and strbuf_git_common_path()
  dir.c: rename str(n)cmp_icase to fspath(n)cmp
2016-05-23 14:54:29 -07:00
Junio C Hamano
40cfc95856 Merge branch 'nd/error-errno'
The code for warning_errno/die_errno has been refactored and a new
error_errno() reporting helper is introduced.

* nd/error-errno: (41 commits)
  wrapper.c: use warning_errno()
  vcs-svn: use error_errno()
  upload-pack.c: use error_errno()
  unpack-trees.c: use error_errno()
  transport-helper.c: use error_errno()
  sha1_file.c: use {error,die,warning}_errno()
  server-info.c: use error_errno()
  sequencer.c: use error_errno()
  run-command.c: use error_errno()
  rerere.c: use error_errno() and warning_errno()
  reachable.c: use error_errno()
  mailmap.c: use error_errno()
  ident.c: use warning_errno()
  http.c: use error_errno() and warning_errno()
  grep.c: use error_errno()
  gpg-interface.c: use error_errno()
  fast-import.c: use error_errno()
  entry.c: use error_errno()
  editor.c: use error_errno()
  diff-no-index.c: use error_errno()
  ...
2016-05-17 14:38:28 -07:00
Nguyễn Thái Ngọc Duy
8d19e93094 builtin/worktree.c: use error_errno()
While at there, improve the error message to say _what_ failed to
remove.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-05-09 12:29:08 -07:00
Nguyễn Thái Ngọc Duy
8d9fdd7087 worktree.c: check whether branch is rebased in another worktree
This function find_shared_symref() is used in a couple places:

1) in builtin/branch.c: it's used to detect if a branch is checked out
   elsewhere and refuse to delete the branch.

2) in builtin/notes.c: it's used to detect if a note is being merged in
   another worktree

3) in branch.c, the function die_if_checked_out() is actually used by
   "git checkout" and "git worktree add" to see if a branch is already
   checked out elsewhere and refuse the operation.

In cases 1 and 3, if a rebase is ongoing, "HEAD" will be in detached
mode, find_shared_symref() fails to detect it and declares "no branch is
checked out here", which is not really what we want.

This patch tightens the test. If the given symref is "HEAD", we try to
detect if rebase is ongoing. If so return the branch being rebased. This
makes checkout and branch delete operations safer because you can't
checkout a branch being rebased in another place, or delete it.

Special case for checkout. If the current branch is being rebased,
git-rebase.sh may use "git checkout" to abort and return back to the
original branch. The updated test in find_shared_symref() will prevent
that and "git rebase --abort" will fail as a result.
find_shared_symref() and die_if_checked_out() have to learn a new
option ignore_current_worktree to loosen the test a bit.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-04-22 14:09:38 -07:00
Ray Zhang
ef2a0ac9a0 worktree: add: introduce --checkout option
By adding this option which defaults to true, we can use the
corresponding --no-checkout to make some customizations before
the checkout, like sparse checkout, etc.

Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Reviewed-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Ray Zhang <zhanglei002@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-03-29 11:12:36 -07:00
Junio C Hamano
11529ecec9 Merge branch 'jk/tighten-alloc'
Update various codepaths to avoid manually-counted malloc().

* jk/tighten-alloc: (22 commits)
  ewah: convert to REALLOC_ARRAY, etc
  convert ewah/bitmap code to use xmalloc
  diff_populate_gitlink: use a strbuf
  transport_anonymize_url: use xstrfmt
  git-compat-util: drop mempcpy compat code
  sequencer: simplify memory allocation of get_message
  test-path-utils: fix normalize_path_copy output buffer size
  fetch-pack: simplify add_sought_entry
  fast-import: simplify allocation in start_packfile
  write_untracked_extension: use FLEX_ALLOC helper
  prepare_{git,shell}_cmd: use argv_array
  use st_add and st_mult for allocation size computation
  convert trivial cases to FLEX_ARRAY macros
  use xmallocz to avoid size arithmetic
  convert trivial cases to ALLOC_ARRAY
  convert manual allocations to argv_array
  argv-array: add detach function
  add helpers for allocating flex-array structs
  harden REALLOC_ARRAY and xcalloc against size_t overflow
  tree-diff: catch integer overflow in combine_diff_path allocation
  ...
2016-02-26 13:37:16 -08:00
Jeff King
3733e69464 use xmallocz to avoid size arithmetic
We frequently allocate strings as xmalloc(len + 1), where
the extra 1 is for the NUL terminator. This can be done more
simply with xmallocz, which also checks for integer
overflow.

There's no case where switching xmalloc(n+1) to xmallocz(n)
is wrong; the result is the same length, and malloc made no
guarantees about what was in the buffer anyway. But in some
cases, we can stop manually placing NUL at the end of the
allocated buffer. But that's only safe if it's clear that
the contents will always fill the buffer.

In each case where this patch does so, I manually examined
the control flow, and I tried to err on the side of caution.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-02-22 14:51:09 -08:00
Nguyễn Thái Ngọc Duy
beb6f24bee worktree add -B: do the checkout test before update branch
If --force is not given but -B is, we should not proceed if the given
branch is already checked out elsewhere. add_worktree() has this test,
but it kicks in too late when "git branch --force" is already
executed. As a result, even though we correctly refuse to create a new
worktree, we have already updated the branch and mess up the other
checkout.

Repeat the die_if_checked_out() test again for this specific case before
"git branch" runs.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-02-15 15:54:13 -08:00
Nguyễn Thái Ngọc Duy
0ebf4a2af3 worktree: fix "add -B"
Current code does not update "symref" when -B is used. This string
contains the new HEAD. Because it's empty "git worktree add -B" fails at
symbolic-ref step.

Because branch creation is already done before calling add_worktree(),
-B is equivalent to -b from add_worktree() point of view. We do not need
the special case for -B.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-02-15 15:49:33 -08:00
brian m. carlson
f2fd0760f6 Convert struct object to object_id
struct object is one of the major data structures dealing with object
IDs.  Convert it to use struct object_id instead of an unsigned char
array.  Convert get_object_hash to refer to the new member as well.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Jeff King <peff@peff.net>
2015-11-20 08:02:05 -05:00
Junio C Hamano
522e2f4515 Merge branch 'es/worktree-add'
* es/worktree-add:
  worktree: usage: denote <branch> as optional with 'add'
2015-10-26 15:55:21 -07:00
Sidhant Sharma
ae9f2745be worktree: usage: denote <branch> as optional with 'add'
Although 1eb07d8 (worktree: add: auto-vivify new branch when
<branch> is omitted, 2015-07-06) updated the documentation when
<branch> became optional, it neglected to update the in-code
usage message. Fix this oversight.

Reported-by: ch3cooli@gmail.com
Signed-off-by: Sidhant Sharma <tigerkid001@gmail.com>
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-10-18 23:35:58 -07:00
Michael Rappazzo
bb9c03b82a worktree: add 'list' command
'git worktree list' iterates through the worktree list, and outputs
details of the worktree including the path to the worktree, the currently
checked out revision and branch, and if the work tree is bare.  There is
also porcelain format option available.

Signed-off-by: Michael Rappazzo <rappazzo@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-10-08 11:57:14 -07:00
Junio C Hamano
d75bb73bcf Merge branch 'jc/am-state-fix'
Recent reimplementation of "git am" changed the format of state
files kept in $GIT_DIR/rebase-apply/ without meaning to do so,
primarily because write_file() API was cumbersome to use and it was
easy to mistakenly make text files with incomplete lines.  Update
write_file() interface to make it harder to misuse.

* jc/am-state-fix:
  write_file(): drop caller-supplied LF from calls to create a one-liner file
  write_file_v(): do not leave incomplete line at the end
  write_file(): drop "fatal" parameter
  builtin/am: make sure state files are text
  builtin/am: introduce write_state_*() helper functions
2015-08-31 15:39:03 -07:00
Junio C Hamano
1f76a10b2d write_file(): drop caller-supplied LF from calls to create a one-liner file
All of the callsites covered by this change call write_file() or
write_file_gently() to create a one-liner file.  Drop the caller
supplied LF and let these callees to append it as necessary.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-25 12:49:19 -07:00
Junio C Hamano
12d6ce1dba write_file(): drop "fatal" parameter
All callers except three passed 1 for the "fatal" parameter to ask
this function to die upon error, but to a casual reader of the code,
it was not all obvious what that 1 meant.  Instead, split the
function into two based on a common write_file_v() that takes the
flag, introduce write_file_gently() as a new way to attempt creating
a file without dying on error, and make three callers to call it.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-24 13:09:02 -07:00
Junio C Hamano
53860f0392 Merge branch 'es/worktree-add-cleanup'
The "new-worktree-mode" hack in "checkout" that was added in
nd/multiple-work-trees topic has been removed by updating the
implementation of new "worktree add".

* es/worktree-add-cleanup: (25 commits)
  Documentation/git-worktree: fix duplicated 'from'
  Documentation/config: mention "now" and "never" for 'expire' settings
  Documentation/git-worktree: fix broken 'linkgit' invocation
  checkout: drop intimate knowledge of newly created worktree
  worktree: populate via "git reset --hard" rather than "git checkout"
  worktree: avoid resolving HEAD unnecessarily
  worktree: make setup of new HEAD distinct from worktree population
  worktree: detect branch-name/detached and error conditions locally
  worktree: add_worktree: construct worktree-population command locally
  worktree: elucidate environment variables intended for child processes
  worktree: make branch creation distinct from worktree population
  worktree: add: suppress auto-vivication with --detach and no <branch>
  worktree: make --detach mutually exclusive with -b/-B
  worktree: introduce options container
  worktree: simplify new branch (-b/-B) option checking
  worktree: improve worktree setup message
  branch: publish die_if_checked_out()
  checkout: teach check_linked_checkout() about symbolic link HEAD
  checkout: check_linked_checkout: simplify symref parsing
  checkout: check_linked_checkout: improve "already checked out" aesthetic
  ...
2015-08-12 14:09:56 -07:00